Re: [PATCH] fix backlight brightness on intel LVDS panel after reopening lid
On Wed, 16 Feb 2011 20:59:35 +0100 Alex Riesen wrote: > On Wed, Feb 16, 2011 at 20:54, Jesse Barnes wrote: > > On Wed, 16 Feb 2011 20:46:45 +0100 > > Alex Riesen wrote: > >> > The backlight level on this Dell XPS M1330 reduces every time I reopen > >> > the lid, and BIOS does not seem to know anything about that (the > >> > keyboard shortcuts to set backlight brightness cause it to jump to the > >> > level next to the one set before closing the lid). > >> > >> It is this bug, I believe: > >> > >> https://bugzilla.kernel.org/show_bug.cgi?id=25072 > >> > >> I somehow missed it at first, and only noticed after sending the patch. > > > > There's also this patch: https://patchwork.kernel.org/patch/499221/. > > It got things working on my E6510 again at least. > > I don't think it is related. I don't have problems switching > the outputs (frankly, didn't try) and I do have problems > restoring backlight, very similar to what I had earlier in > 2.6.37. Right, but it affects the registration of the backlight and ACPI video interface as well, so can affect backlight restore on resume. In my case, without the above patch my backlight wouldn't be restored on resume, so I'd have to manually echo a value into /sys/class/backlight/ to get my display back. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fix backlight brightness on intel LVDS panel after reopening lid
On Sat, 19 Feb 2011 15:07:49 -0800 Linus Torvalds wrote: > On Sat, Feb 19, 2011 at 4:26 AM, Alex Riesen wrote: > > On Sat, Feb 19, 2011 at 13:11, Alex Riesen wrote: > >>> Lastly, could you verify that my patch at > >>> https://lkml.org/lkml/2011/2/16/447 fixes > >>> it for you too? (Make sure you're at max brightness before rebooting.) > >> > >> I'll try it now. > >> > > > > I can confirm that it does fix backlight in my case (Dell XPS 1330, > > LVDS panel, GM965/GL960). > > > > Tested-by: Alex Riesen > > Guys, should I apply this, or will I get it through somebody's pull? I'm worried that removing combo mode will break some working setups, but if it's seen a lot of testing and is ok, then I'm fine with it, as it definitely simplifies things. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: vblank problem (and proposed fix) on crtc > 1
On Thu, 3 Mar 2011 17:34:53 -0600 (CST) Ilija Hadzic wrote: > The fix/improvement I propose is to extend the request.type field > in drmVBlank structure with additional 5 bits that I call high_crtc > (there are lots of unused bits in that field). 5 bits covers for 32 > CRTCs, which seems to be the hard limit imposed by various parts of the > Xorg and DDX (e.g. possible_crtcs mask in the display descriptor, > and the like). If high_crtc is zero, then DRM (kernel module) > looks at the primary/secondary flag and maps them to crtc 0 and 1 > (backwards compatibility with older DDX or DDX for other device > that does not use the new high_crtc field). If it's not zero then it > means that the higher CRTC number is specified by DDX (i.e. userland > is a new DDX) and vblank is waited on the specified CRTC (this is used > only for crtc > 1, crtc 0 and 1, still use the old style). Yeah, I think that should work, though another option would be to just add a new ioctl. That would make compat checking easy for new code; it could just call the new ioctl and if that returned -ENOTTY it could fall back to the old one and throw away the CRTC info or complain if the count was too high. But you're right that when we re-wrote the code we fixed it to handle > 2 CRTCs, so it should be mostly ready for that (modulo testing, which it sounds like you're doing already). Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm/i915: Fix DPMS and suspend interaction for intel_panel.c
On Fri, 11 Mar 2011 02:35:45 +0100 (CET)
"Indan Zupancic" wrote:
> drm/i915: Fix DPMS and suspend interaction for intel_panel.c
>
> When suspending intel_panel_disable_backlight() is never called,
> but intel_panel_enable_backlight() is called at resume. With the
> effect that if the brightness was ever changed after screen
> blanking, the wrong brightness gets restored at resume time.
>
> Nothing guarantees that those calls will be balanced, so having
> backlight_enabled makes no sense, as the real state can change
> without the panel code noticing. So keep things as stateless as
> possible.
>
> Signed-off-by: Indan Zupancic
Chris is right that we don't always control the backlight brightness
directly, so we'll want a more complete solution at any rate.
I don't think this one is urgent enough to send upstream now, and it
would be good to make a couple of other fixes as well, while you're
fixing things up. :) Comments below.
> -/* i915_suspend.c */
> -extern int i915_save_state(struct drm_device *dev);
> -extern int i915_restore_state(struct drm_device *dev);
> -
Looks like a spurious cleanup hunk, should be a separate hunk (possibly
along with other save/restore state cleanups, like removing all the
ILK+ code that probably won't work well :).
> -void intel_panel_setup_backlight(struct drm_device *dev)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - dev_priv->backlight_level = intel_panel_get_backlight(dev);
> - dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
> }
This is getting pretty messy. Your patch is a step in the right
direction, but I think we may as well go further:
- suspend/resume of backlight state belongs in the backlight class
driver
- that driver should call into the registered i915 backlight handler
if i915 is controlling it natively (PWM native, combo, legacy)
- we need a backlight driver struct with function pointers for the
various calls, so we can split out the PCH functions from the rest;
might be good to separate native, combo, and legacy that way as
well; even if results in some code duplication it might make each
easier to read and less likely to break others when we make changes
Any thoughts about that? I think Chris and Matthew have been talking
about it as well, so I'd be curious what our backlight final solution
ought to be.
Thanks,
Jesse
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Hold the mode mutex whilst probing for sysfs status
On Tue, 15 Mar 2011 11:40:00 +
Chris Wilson wrote:
> As detect will use hw registers and may modify structures, it needs to be
> serialised by use of the dev->mode_config.mutex. Make it so.
>
> Otherwise, we may cause random crashes as the sysfs file is queried
> whilst a concurrent hotplug poll is being run. For example:
>
> [ 1189.189626] BUG: unable to handle kernel NULL pointer dereference at
> 0100
> [ 1189.189821] IP: [] intel_tv_detect_type+0xa2/0x203 [i915]
> [ 1189.190020] *pde =
> [ 1189.190104] Oops: [#1] SMP
> [ 1189.190209] last sysfs file:
> /sys/devices/pci:00/:00:02.0/drm/card0/card0-SVIDEO-1/status
> [ 1189.190412] Modules linked in: mperf cpufreq_conservative
> cpufreq_userspace cpufreq_powersave cpufreq_stats decnet uinput fuse loop
> joydev snd_hd a_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep
> snd_pcm_oss snd_mixer_oss snd_pcm i915 snd_seq_midi snd_rawmidi
> snd_seq_midi_event snd_seq drm_kms_helper snd_timer uvcvideo d rm
> snd_seq_device eeepc_laptop tpm_tis usbhid videodev i2c_algo_bit v4l1_compat
> snd sparse_keymap i2c_core hid serio_raw tpm psmouse evdev tpm_bios rfkill
> shpchp ac processor rng_c ore battery video power_supply soundcore
> pci_hotplug button output snd_page_alloc usb_storage uas ext3 jbd mbcache
> sd_mod crc_t10dif ata_generic ahci libahci ata_piix libata uhci_h cd ehci_hcd
> scsi_mod usbcore thermal atl2 thermal_sys nls_base [last unloaded:
> scsi_wait_scan]
> [ 1189.192007]
> [ 1189.192007] Pid: 1464, comm: upowerd Not tainted 2.6.37-2-686 #1 ASUSTeK
> Computer INC. 701/701
> [ 1189.192007] EIP: 0060:[] EFLAGS: 00010246 CPU: 0
> [ 1189.192007] EIP is at intel_tv_detect_type+0xa2/0x203 [i915]
> [ 1189.192007] EAX: EBX: dca74000 ECX: e0f68004 EDX: 00068004
> [ 1189.192007] ESI: dd110c00 EDI: 400c0c37 EBP: dca7429c ESP: de365e2c
> [ 1189.192007] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
> [ 1189.192007] Process upowerd (pid: 1464, ti=de364000 task=dcc8acb0
> task.ti=de364000)
> [ 1189.192007] Stack: Mar 15 03:43:23 hostname kernel: [ 1189.192007]
> e0c2cda4 7000 400c0c30 dd111000 de365e54 de365f24 dd110c00
> [ 1189.192007] e0c22203 0100 0003
> 4353544e
> [ 1189.192007] 30383420 0069
>
> [ 1189.192007] Call Trace: Mar 15 03:43:23 hostname kernel: [ 1189.192007]
> [] ? intel_tv_detect+0x89/0x12d [i915]
> [ 1189.192007] [] ? status_show+0x0/0x2f [drm]
> [ 1189.192007] [] ? status_show+0x14/0x2f [drm]
>
> [Digression: what is upowerd doing reading those power hungry files?]
>
> Reported-by: Paul Menzel
> Signed-off-by: Chris Wilson
> Cc: sta...@kernel.org
> ---
> drivers/gpu/drm/drm_sysfs.c |7 +++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 85da4c4..2eee8e0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -158,8 +158,15 @@ static ssize_t status_show(struct device *device,
> {
> struct drm_connector *connector = to_drm_connector(device);
> enum drm_connector_status status;
> + int ret;
> +
> + ret = mutex_lock_interruptible(&connector->dev->mode_config.mutex);
> + if (ret)
> + return ret;
>
> status = connector->funcs->detect(connector, true);
> + mutex_unlock(&connector->dev->mode_config.mutex);
> +
How about adding a mutex assertion check in the detect hook as well? I
think we need a more generous sprinkling of those around the CRTC
helper code in general...
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] xf86-video-ati: vblank wait on crtc > 1
On Fri, 18 Mar 2011 16:58:39 -0500 (CDT) Ilija Hadzic wrote: > > Hi Alex, > > Below is a patch against the master branch of xf86-video-ati that adds > support for waits on vblank events on CRTCs that are greater than 1 (and > thus cannot be represented using current primary/secondary flags > interface). The patch makes use of GET_CAP ioctl to check whether > vblanks on crtc > 1 are supported by the kernel. If they are not > falls back to legacy behavior. Otherwise, it uses correct crtc numbers > when waiting on vblank and thus corrects the problem seen in certain > multiscreen configurations. > > The issue was discussed on the dri-devel list in these two threads > > http://lists.freedesktop.org/archives/dri-devel/2011-March/009009.html > http://lists.freedesktop.org/archives/dri-devel/2011-March/009025.html > The duplicated code in each function is begging to get pulled out into a separate function... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel/drm: vblank wait on crtc > 1
On Fri, 18 Mar 2011 16:58:04 -0500 (CDT) Ilija Hadzic wrote: > > Hi Dave, > > Below is a patch against drm-next branch of 2.6.38-rc8+ kernel that adds > the capability to wait on vblank events for CRTCs that are greater than 1 > and thus cannot be represented with primary/secondary flags in the legacy > interface. It was discussed on the dri-devel list in these two threads: > > http://lists.freedesktop.org/archives/dri-devel/2011-March/009009.html > http://lists.freedesktop.org/archives/dri-devel/2011-March/009025.html > > This patch extends the interface to drm_wait_vblank ioctl so that crtc>1 > can be represented. It also adds a new capability to drm_getcap ioctl so > that the user space can check whether the new interface to drm_wait_vblank > is supported (and fall back to the legacy interface if not) I like the new param check, but I'd still prefer a new ioctl to abusing the old one. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] kernel/drm: vblank wait on crtc > 1
On Fri, 18 Mar 2011 18:13:11 -0500 (CDT) Ilija Hadzic wrote: > > > On Fri, 18 Mar 2011, Jesse Barnes wrote: > > > > > I like the new param check, but I'd still prefer a new ioctl to abusing > > the old one. > > > > It's not "abusing" it but extending the interface in a > backwards-compatible manner. Introducing a new one would result in two > ioctls that essentially do the same thing, which I don't like. Yes abusing was a strong word; I just don't like encoding crtc numbers in a bitfield, when we just use ints everywhere else. Not a big deal, Dave will make the final call. Thanks for doing this work. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Future desktop on dumb frame buffers?
On Sat, 19 Mar 2011 12:20:24 +0100 Geert Uytterhoeven wrote: > As noone responded to my question in > http://www.spinics.net/lists/dri-devel/msg08851.html > (yes, it was a bit hidden in a thread), I'm asking it here again (and > also on the Wayland > mailing list). > > Basically I'm still puzzled about this KMS thing. If the name "Kernel > Mode Setting" > covers it, then how does it compare to plain fbdev? Just additional frame > buffer > memory management? > Also, some people may remember we did have kernel messages (e.g. oops, panic) > on graphical consoles with fbdev, until people started not liking them > showing up > on their X desktops... We support panic these days as well, but people still don't like seeing them. :) The DRM KMS APIs provide everything fbdev provides, plus memory management, a way to expose acceleration (via GEM or TTM), and a way to manage multiple outputs reasonably. > Furthermore, everybody states that "future desktop" (that's > http://wayland.freedesktop.org/) > will require KMS drivers. > How do/will we handle this on dumb frame buffers? It's not like we can't do > "advanced" things like compositing using the CPU. Transparency may stretch > it a bit on lower end CPUs, but you don't always need that. There's nothing in DRM that precludes doing simple fbdev-like drivers as well, though for many embedded uses I wouldn't expect it to provide a whole lot of benefit. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Future desktop on dumb frame buffers?
On Mon, 21 Mar 2011 19:19:43 + timofonic timofonic wrote: > So if KMS is so cool and provides many advantages over fbdev and > such... Why isn't more widely used intead of still relying on fbdev? > Why still using fbdev emulation (that is partial and somewhat broken, > it seems) instead using KMS directly? Used by what? All three major GPU device classes have KMS support (Intel, ATI, and nVidia). If you want it for a particular device, you can always port it over. As for fbdev emulation, what's still using it? There's nothing stopping projects from converting over; X and Wayland can already handle KMS APIs just fine. > I know the graphic driver situation is quite bad on Linux, especially > on the embedded world. Fbdev seems is still quite used there by binary > blob drivers. Probably for a couple of reasons: 1) inertia: fbdev has been around a lot longer, and provides most of what embedded devices need anyway 2) feature set: why bother doing a full KMS driver if you're not going to use any of the additional features it would provide (output management, memory management, execution management) Jesse ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Future desktop on dumb frame buffers?
On Mon, 21 Mar 2011 20:50:20 +0100 Geert Uytterhoeven wrote: > On Mon, Mar 21, 2011 at 20:25, Jesse Barnes wrote: > > On Mon, 21 Mar 2011 19:19:43 + > > timofonic timofonic wrote: > >> So if KMS is so cool and provides many advantages over fbdev and > >> such... Why isn't more widely used intead of still relying on fbdev? > >> Why still using fbdev emulation (that is partial and somewhat broken, > >> it seems) instead using KMS directly? > > > > Used by what? All three major GPU device classes have KMS support > > (Intel, ATI, and nVidia). If you want it for a particular device, you > > can always port it over. > > The three major GPU device classes on PC... Yes, good point. :) > > As for fbdev emulation, what's still using it? There's nothing > > stopping projects from converting over; X and Wayland can already > > handle KMS APIs just fine. > > Can Wayland handle fbdev APIs ... Yes. Fundamentally, the Wayland protocol just assumes a way to share buffers between processes. For the software raster version of the Qt port, Kristian created a shmem interface for doing that to allow the results of CPU rendering to be passed around without copying. On an embedded device that would be one way to go. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Future desktop on dumb frame buffers?
On Mon, 21 Mar 2011 12:34:38 -0700 Corbin Simpson wrote: > Related: We are still missing basic userspace tools (kmsset, e.g.), > some kind of direct KMS console (kmscon would work, if it existed), > and an xf86-video-modesetting which compiles and works (this is > actually possible now, with some patches that landed in 2.6.38 for > generic KMS access.) Yeah, we used to call that drmcon, and it's still a big open. I think there are some projects that sit on top of fbdev and provide a good text console with fancy character and input support, but I don't know if any of them have been ported to KMS to handle multiple outputs or with an aim toward integrating into a distro as a VT replacement. kmsset or something would be pretty easy to do; the modetest program in the drm repo would be a good starting point for that. One limitation there is handling fbcon, which makes reallocation of the framebuffer somewhat difficult. IIRC plymouth or whatever Fedora is using these days uses the KMS APIs though... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Future desktop on dumb frame buffers?
On Mon, 21 Mar 2011 21:20:08 + Alan Cox wrote: > > 1) inertia: fbdev has been around a lot longer, and provides most of > > what embedded devices need anyway > > 2) feature set: why bother doing a full KMS driver if you're not > > going to use any of the additional features it would provide (output > > management, memory management, execution management) > > 3) its got documentation Jeez, some people want it all. You looking for docs for the ioctls and such? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] i915: Remove pipe A force quirk for 855GM and 845G
On Tue, 22 Mar 2011 03:05:45 + Ben Hutchings wrote: > On Mon, 2011-03-21 at 07:38 +, Chris Wilson wrote: > > On Sun, 20 Mar 2011 23:07:04 +, Ben Hutchings > > wrote: > > > Applying this quirk to the 855GM in all systems causes regressions > > > (Debian bugs #493096, #619019). Instead, apply the quirk to specific > > > models as listed in the old X driver. > > > > > > I don't see any explanation for this quirk being applied to the 845G, > > > except perhaps that VT switching used to hang if pipe A was turned > > > off. However, that seems to be a problem only when using UMS. So > > > remove the quirk for the 845G as well. > > > > The quirk should only be required for 830M due to the numerous instances > > where a unit on the second pipe is actually wired into the clock on the > > first pipe. (And so it is easiest to keep the first pipe active at all > > times.) > > When you say 'wired into', is this part of the chip design or something > done on the board? > > Jesse, why did you add the quirk for other chips? The DDX driver had an option to force enable pipe A, and we told people to report bugs when they needed it. So we got a bunch from Ubuntu and elsewhere and added them without too much investigation into the root cause (as you say below it could have been something else). > > I'd prefer the quirk table to disappear and simply be replaced by > > IS_830M(). However, that requires testing and so should only be done > > piecemeal. And leaves some doubt as to why the other machines were in the > > quirk table in the first place. > > The commit messages referring to VT switching suggest that the problems > related to disabling part A may actually have been related to handover > to the console driver before KMS. Yes, definitely possible. We didn't have all the assertion checks we have now, so we may have just been masking other problems without knowing it. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH] HDMI:Support for EDID parsing in kernel.
On Wed, 23 Mar 2011 18:58:27 +0530 "K, Mythri P" wrote: > Hi Dave, > > On Wed, Mar 23, 2011 at 6:16 AM, Dave Airlie wrote: > > On Wed, Mar 23, 2011 at 3:32 AM, Mythri P K wrote: > >> Adding support for common EDID parsing in kernel. > >> > >> EDID - Extended display identification data is a data structure provided by > >> a digital display to describe its capabilities to a video source, This a > >> standard supported by CEA and VESA. > >> > >> There are several custom implementations for parsing EDID in kernel, some > >> of them are present in fbmon.c, drm_edid.c, sh_mobile_hdmi.c, Ideally > >> parsing of EDID should be done in a library, which is agnostic of the > >> framework (V4l2, DRM, FB) which is using the functionality, just based on > >> the raw EDID pointer with size/segment information. > >> > >> With other RFC's such as the one below, which tries to standardize HDMI > >> API's > >> It would be better to have a common EDID code in one place.It also helps to > >> provide better interoperability with variety of TV/Monitor may be even by > >> listing out quirks which might get missed with several custom > >> implementation > >> of EDID. > >> http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/30401 > >> > >> This patch tries to add functions to parse some portion EDID (detailed > >> timing, > >> monitor limits, AV delay information, deep color mode support, Audio and > >> VSDB) > >> If we can align on this library approach i can enhance this library to > >> parse > >> other blocks and probably we could also add quirks from other > >> implementation > >> as well. > >> > > > > If you want to take this approach, you need to start from the DRM EDID > > parser, > > its the most well tested and I can guarantee its been plugged into more > > monitors > > than any of the others. There is just no way we would move the DRM parser > > to a > > library one that isn't derived from it + enhancements, as we'd throw away > > the > > years of testing and the regression count would be way too high. > > > I had a look at the DRM EDID code, but for quirks it looks pretty much the > same. > yes i could take quirks and other DRM tested code and enhance, but > still the code has to do away with struct drm_display_mode > which is very much custom to DRM. If that's the only issue you have, we could easily rename that structure or add conversion funcs to a smaller structure if that's what you need. Dave's point is that we can't ditch the existing code without introducing a lot of risk; it would be better to start a library-ized EDID codebase from the most complete one we have already, i.e. the DRM EDID code. Do you really think the differences between your code and the existing DRM code are irreconcilable? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm next tree
On Wed, 23 Mar 2011 08:21:53 -0400 Stephen Clark wrote: > On 03/22/2011 10:19 PM, Linus Torvalds wrote: > > So I had hoped - yes, very naïve of me, I know - that this merge > > window would be different. > > > > But it's not. > > > > On Wed, Mar 16, 2011 at 9:09 PM, Dave Airlie wrote: > > > >> i915: big 855 fix, lots of output setup refactoring, lots of misc fixes. > >> > > .. and apparently a lot of breakage too. My crappy laptop that I abuse > > for travel is - once more - broken by the updates. I cannot suspend > > and resume, because every resume seems to fail. > > > > One of the more useful failures was: > > > > [ 61.656055] [drm:i915_hangcheck_elapsed] *ERROR* Hangcheck timer > > elapsed... GPU hung > > [ 61.656079] [drm] capturing error event; look for more information > > in /debug/dri/0/i915_error_state > > [ 61.664387] [drm:i915_wait_request] *ERROR* i915_wait_request > > returns -11 (awaiting 2 at 0, next 3) > > > > and I'm attaching the error_state file from that particular case here. > > In other cases it seems to just hang entirely. > > > > Keith/Jesse/Chris - I don't know that it's i915, and it will take > > forever to bisect (I'll try). But it does seem pretty likely. > > > > Linus > > > Why can't the gpu be reset/restarted when this happens? When a nic card > gets hung it is reinitialized > and restarted why not the gpu? Yeah, we try to restart in this case, but often just end up back in the same situation when the app runs again. We could be meaner about things and SIGILL the app, but often it's an innocent bystander, and the real problem is kernel object synchronization and/or the DRI driver generating bad commands. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm next tree
On Wed, 23 Mar 2011 08:29:35 -0700 Linus Torvalds wrote: > On Tue, Mar 22, 2011 at 7:19 PM, Linus Torvalds > wrote: > > > > Keith/Jesse/Chris - I don't know that it's i915, and it will take > > forever to bisect (I'll try). But it does seem pretty likely. > > Ok, so I'm still bisecting, but it's definitely the DRM pull. Current > bisection log attached (the result does contain the fbdev pull from > Paul Mundt, but that doesn't touch any files I compile afaik). Chris mentioned a7a75c8f7 on irc, not sure if it was regarding this issue though, but it does seem a likely candidate. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes
On Fri, 25 Mar 2011 10:01:14 -0400 Jerome Glisse wrote: > On Fri, Mar 25, 2011 at 6:25 AM, Ben Skeggs wrote: > > Oh, I wish this were actually the case. The last time we attempted such a > > thing we were blasted by Linus. It does make me wonder at why we're even > > bothering being in staging. > > > > This is where the binary drivers have a huge advantage, they package all > > the pieces of their driver together and can modify things as necessary. > > > > Part of me does think such an approach with the open source graphics > > drivers would be better. The current model doesn't really fit too well in > > my opinion. Though, admittedly, there's different problems to going other > > ways. > > > > Ben. > > Well i think being able to evolve the API would help a lot, it should > still be possible to keep supporting old API over a year or so. But my > feeling is some of the current API for some of the device, needs heavy > lifting if we ever want to improve things. Going back to the old model of a separate drm repo would create more problems than it solves, IMO. One thing I think Linus has been fairly consistent about is that making things easy for users (well at least power users who build their own kernels) is important. That means ABIs need to be stable so that their existing userspace continues to work even after a kernel upgrade. If we went back to the old, out of tree model, we might be able to break things more easily (i.e. require lockstep upgrades of kernel & userspace when we change things, hopefully for a good reason), but I think end users would end up suffering. They'd have to rely on their distro to pick up such changes and make sure dependencies were met and that upgrades/downgrades worked properly across the pre-packaged kernels that they made available. One of the main reasons we moved in-tree was to make sure our bits got into the hands of users more quickly, and to make life easier for the various downstream distros, not all of which have deep expertise in the graphics stack. Fortunately, neither of the issues that started this thread, the suspend/resume regression in i915 and the vblank ioctl enhancement, are problems in this respect. The former is just a bug (though definitely not one that should have made it to Linus's tree) and the latter does preserve compatibility and fix a major issue, so isn't really a problem imo. But in the general sense, I think we just have to bite the bullet, take our time with ABI additions and changes so as to preserve compatibility for a long time (I think we've been doing well with this on the Intel side at least; we add feature flags every time we change something, and make sure userspace is forward and backward compatible). This is more work for us, but I think it benefits the user in the end. And it could be worse, at least we're not still dealing with memory layout compatibility between the DRM, fbdev, DDX and DRI drivers anymore! -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm fixes
On Fri, 25 Mar 2011 10:04:10 -0400 Jerome Glisse wrote: > My feeling on that is that maybe too much code sharing accross gpu of > different generation hurt more than it helps. I have got the feeling > that some of the newer Intel asic share some of the bit of older one > and that intel is focusing there attention on newer one and obviously > doesn't have time or resource to check that change they do don't > impact older hw (i don't think such testing is doable without massive > investment which is very very unlikely to happen given size of linux > market). God yes. The mantra of "code sharing is always good" sounds nice, but in my experience it's very often not true, especially when you're still trying to get things to work. We've been slowly splitting code out to make it more robust against changes to different generations, but we still have a little ways to go. And of course, such splitting introduces problems in itself. But as a policy going forward, I'll advocate splitting code whenever a new generation requires something slightly different than an old one. As an example, rather than adding a few conditionals to our FDI training code to support newer chips, I've split it into separate functions entirely, leaving the old one alone. If, after awhile we've decided that they really are mostly the same and we have things working well, we can consider merging them again, but only after extensive testing across generations. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Try a bit harder to get output on the screen at panic time
On Fri, 9 Apr 2010 15:10:50 -0700 Jesse Barnes wrote: > This set of 3 patches makes it a little more likely we'll get panic > output onto the screen even when X is running, assuming a KMS enabled > stack anyway. > > It gets me from a blank or very sparsely populated black screen at > panic time, to one including the full backtrace and panic output at > panic time (tested with "echo c > /proc/sysrq-trigger" from an X > session). > > It doesn't cover every case; for instance I think it'll fail when X has > disabled the display, but those cases need to be handled with separate > patches anyway (need to add atomic DPMS paths for instance). > > Anyway, please test these out and let me know if they work for you. Linus, did you get a chance to try these at all? They're small, and if they work for you I thought maybe they had a chance to get into 2.6.34. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Try a bit harder to get output on the screen at panic time
On Fri, 9 Apr 2010 15:10:50 -0700 Jesse Barnes wrote: > This set of 3 patches makes it a little more likely we'll get panic > output onto the screen even when X is running, assuming a KMS enabled > stack anyway. > > It gets me from a blank or very sparsely populated black screen at > panic time, to one including the full backtrace and panic output at > panic time (tested with "echo c > /proc/sysrq-trigger" from an X > session). > > It doesn't cover every case; for instance I think it'll fail when X has > disabled the display, but those cases need to be handled with separate > patches anyway (need to add atomic DPMS paths for instance). > > Anyway, please test these out and let me know if they work for you. Ping Linus & Dave again. Have you guys tried these? Really, it's cool. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Try a bit harder to get output on the screen at panic time
On Thu, 20 May 2010 04:27:07 +0300 Maxim Levitsky wrote: > On Thu, 2010-05-20 at 04:13 +0300, Maxim Levitsky wrote: > > On Wed, 2010-05-19 at 17:34 -0700, Jesse Barnes wrote: > > > On Fri, 9 Apr 2010 15:10:50 -0700 > > > Jesse Barnes wrote: > > > > > > > This set of 3 patches makes it a little more likely we'll get panic > > > > output onto the screen even when X is running, assuming a KMS enabled > > > > stack anyway. > > > > > > > > It gets me from a blank or very sparsely populated black screen at > > > > panic time, to one including the full backtrace and panic output at > > > > panic time (tested with "echo c > /proc/sysrq-trigger" from an X > > > > session). > > > > > > > > It doesn't cover every case; for instance I think it'll fail when X has > > > > disabled the display, but those cases need to be handled with separate > > > > patches anyway (need to add atomic DPMS paths for instance). > > > > > > > > Anyway, please test these out and let me know if they work for you. > > > > > > Ping Linus & Dave again. Have you guys tried these? Really, it's cool. > > > > > Second that, just tested these patches, and these work perfectly. > > One more reason for me to dump nvidia driver for nouveau. > > > Unfortunately I spoke too soon. > > > After suspend to ram, system doesn't properly resume now. > > My system is based on nvidia G86, I use latest nouveau drivers, and > suspend with compiz running. > > I also patched kernel not to do vt switch on suspend/resume: > > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c > b/drivers/gpu/drm/nouveau/nouveau_state.c > index 062b7f6..b3ef08b 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_state.c > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c > @@ -31,6 +31,7 @@ > #include "drm_crtc_helper.h" > #include > #include > +#include > > #include "nouveau_drv.h" > #include "nouveau_drm.h" > @@ -771,6 +772,8 @@ int nouveau_load(struct drm_device *dev, unsigned > long flags) > int ret = nouveau_card_init(dev); > if (ret) > return ret; > + > + pm_set_vt_switch(0); > } > > return 0; Hm I don't see how my patches would have affected suspend/resume, since they just add "oops_in_progress" checks to a few places. Are you sure something else isn't breaking your resume path? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Try a bit harder to get output on the screen at panic time
On Sat, 22 May 2010 00:57:30 +0300 Maxim Levitsky wrote: > On Fri, 2010-05-21 at 00:14 +0300, Maxim Levitsky wrote: > > On Thu, 2010-05-20 at 09:28 -0700, Jesse Barnes wrote: > > > On Thu, 20 May 2010 04:27:07 +0300 > > > Maxim Levitsky wrote: > > > > > > > On Thu, 2010-05-20 at 04:13 +0300, Maxim Levitsky wrote: > > > > > On Wed, 2010-05-19 at 17:34 -0700, Jesse Barnes wrote: > > > > > > On Fri, 9 Apr 2010 15:10:50 -0700 > > > > > > Jesse Barnes wrote: > > > > > > > > > > > > > This set of 3 patches makes it a little more likely we'll get > > > > > > > panic > > > > > > > output onto the screen even when X is running, assuming a KMS > > > > > > > enabled > > > > > > > stack anyway. > > > > > > > > > > > > > > It gets me from a blank or very sparsely populated black screen at > > > > > > > panic time, to one including the full backtrace and panic output > > > > > > > at > > > > > > > panic time (tested with "echo c > /proc/sysrq-trigger" from an X > > > > > > > session). > > > > > > > > > > > > > > It doesn't cover every case; for instance I think it'll fail when > > > > > > > X has > > > > > > > disabled the display, but those cases need to be handled with > > > > > > > separate > > > > > > > patches anyway (need to add atomic DPMS paths for instance). > > > > > > > > > > > > > > Anyway, please test these out and let me know if they work for > > > > > > > you. > > > > > > > > > > > > Ping Linus & Dave again. Have you guys tried these? Really, it's > > > > > > cool. > > > > > > > > > > > Second that, just tested these patches, and these work perfectly. > > > > > One more reason for me to dump nvidia driver for nouveau. > > > > > > > > > > > > Unfortunately I spoke too soon. > > > > > > > > > > > > After suspend to ram, system doesn't properly resume now. > > > > > > > > My system is based on nvidia G86, I use latest nouveau drivers, and > > > > suspend with compiz running. > > > > > > > > I also patched kernel not to do vt switch on suspend/resume: > > > > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c > > > > b/drivers/gpu/drm/nouveau/nouveau_state.c > > > > index 062b7f6..b3ef08b 100644 > > > > --- a/drivers/gpu/drm/nouveau/nouveau_state.c > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c > > > > @@ -31,6 +31,7 @@ > > > > #include "drm_crtc_helper.h" > > > > #include > > > > #include > > > > +#include > > > > > > > > #include "nouveau_drv.h" > > > > #include "nouveau_drm.h" > > > > @@ -771,6 +772,8 @@ int nouveau_load(struct drm_device *dev, unsigned > > > > long flags) > > > > int ret = nouveau_card_init(dev); > > > > if (ret) > > > > return ret; > > > > + > > > > + pm_set_vt_switch(0); > > > > } > > > > > > > > return 0; > > > > > > Hm I don't see how my patches would have affected suspend/resume, since > > > they just add "oops_in_progress" checks to a few places. Are you sure > > > something else isn't breaking your resume path? > > I am sure. I just reverted them, and everything works again. > > I refer to 3 patches in this thread. > In fact I might look a bit silly, but I applied these patches on top of > linus master tree + nouveau master, and suspend to ram works just fine. > > Maybe it shows up when kgdb+kdb isn't compiled in or so. > Maybe it just triggered some bug in nouveau drivers... > > > (Note that I also enabled kgdb, and kdb, and breaking into kdb (SysRQ+g) > doesn't switch console mode, just hangs till I press 'g'. Ok so it sounds like these particular patches are innocent? As for kdb, I think the latest tree is probably missing the graphics switch support on the driver side... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] Try a bit harder to get output on the screen at panic time
I'll test again this week. I still don't see how these small patches could cause issues with suspend/resume unless we set oops_in_progress during that time on your machine... Jesse Maxim Levitsky wrote: >On Sat, 2010-05-22 at 01:26 +0300, Maxim Levitsky wrote: >> On Fri, 2010-05-21 at 15:02 -0700, Jesse Barnes wrote: >> > On Sat, 22 May 2010 00:57:30 +0300 >> > Maxim Levitsky wrote: >> > >> > > On Fri, 2010-05-21 at 00:14 +0300, Maxim Levitsky wrote: >> > > > On Thu, 2010-05-20 at 09:28 -0700, Jesse Barnes wrote: >> > > > > On Thu, 20 May 2010 04:27:07 +0300 >> > > > > Maxim Levitsky wrote: >> > > > > >> > > > > > On Thu, 2010-05-20 at 04:13 +0300, Maxim Levitsky wrote: >> > > > > > > On Wed, 2010-05-19 at 17:34 -0700, Jesse Barnes wrote: >> > > > > > > > On Fri, 9 Apr 2010 15:10:50 -0700 >> > > > > > > > Jesse Barnes wrote: >> > > > > > > > >> > > > > > > > > This set of 3 patches makes it a little more likely we'll >> > > > > > > > > get panic >> > > > > > > > > output onto the screen even when X is running, assuming a >> > > > > > > > > KMS enabled >> > > > > > > > > stack anyway. >> > > > > > > > > >> > > > > > > > > It gets me from a blank or very sparsely populated black >> > > > > > > > > screen at >> > > > > > > > > panic time, to one including the full backtrace and panic >> > > > > > > > > output at >> > > > > > > > > panic time (tested with "echo c > /proc/sysrq-trigger" from >> > > > > > > > > an X >> > > > > > > > > session). >> > > > > > > > > >> > > > > > > > > It doesn't cover every case; for instance I think it'll fail >> > > > > > > > > when X has >> > > > > > > > > disabled the display, but those cases need to be handled >> > > > > > > > > with separate >> > > > > > > > > patches anyway (need to add atomic DPMS paths for instance). >> > > > > > > > > >> > > > > > > > > Anyway, please test these out and let me know if they work >> > > > > > > > > for you. >> > > > > > > > >> > > > > > > > Ping Linus & Dave again. Have you guys tried these? Really, >> > > > > > > > it's cool. >> > > > > > > > >> > > > > > > Second that, just tested these patches, and these work perfectly. >> > > > > > > One more reason for me to dump nvidia driver for nouveau. >> > > > > > >> > > > > > >> > > > > > Unfortunately I spoke too soon. >> > > > > > >> > > > > > >> > > > > > After suspend to ram, system doesn't properly resume now. >> > > > > > >> > > > > > My system is based on nvidia G86, I use latest nouveau drivers, and >> > > > > > suspend with compiz running. >> > > > > > >> > > > > > I also patched kernel not to do vt switch on suspend/resume: >> > > > > > >> > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c >> > > > > > b/drivers/gpu/drm/nouveau/nouveau_state.c >> > > > > > index 062b7f6..b3ef08b 100644 >> > > > > > --- a/drivers/gpu/drm/nouveau/nouveau_state.c >> > > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c >> > > > > > @@ -31,6 +31,7 @@ >> > > > > > #include "drm_crtc_helper.h" >> > > > > > #include >> > > > > > #include >> > > > > > +#include >> > > > > > >> > > > > > #include "nouveau_drv.h" >> > > > > > #include "nouveau_drm.h" >> > > > > > @@ -771,6 +772,8 @@ int nouveau_load(struct drm_device *dev, >> > > > > > unsigned >> > > > > >
Re: [RFC] Try a bit harder to get output on the screen at panic time
On Sun, 6 Jun 2010 17:36:24 +0100 (BST) James Simmons wrote: > > > On Fri, 9 Apr 2010 15:10:50 -0700 > > Jesse Barnes wrote: > > > > > This set of 3 patches makes it a little more likely we'll get panic > > > output onto the screen even when X is running, assuming a KMS enabled > > > stack anyway. > > > > > > It gets me from a blank or very sparsely populated black screen at > > > panic time, to one including the full backtrace and panic output at > > > panic time (tested with "echo c > /proc/sysrq-trigger" from an X > > > session). > > > > > > It doesn't cover every case; for instance I think it'll fail when X has > > > disabled the display, but those cases need to be handled with separate > > > patches anyway (need to add atomic DPMS paths for instance). > > > > > > Anyway, please test these out and let me know if they work for you. > > > > Ping Linus & Dave again. Have you guys tried these? Really, it's cool. > > Is this safe with UMS and vgacon/sticon? With a oops while using vgacon > you can now touch the hardware buffer while not in vga text mode. Seems unlikely that poking VGA memory at that point would cause data corruption. There shouldn't be any other harm either, since the system is already wedged... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch] i915: take struct_mutex in i915_dma_cleanup()
On Wed, 23 Jun 2010 13:19:55 +0200 Dan Carpenter wrote: > intel_cleanup_ring_buffer() calls drm_gem_object_unreference() (as > opposed to drm_gem_object_unreference_unlocked()) so it needs to be > called with "struct_mutex" held. If we don't hold the lock, it triggers > a BUG_ON(!mutex_is_locked(&dev->struct_mutex)); > > I also audited the other places that call intel_cleanup_ring_buffer() > and they all hold the lock so they're OK. > > This was introduced in: 8187a2b70e3 "drm/i915: introduce > intel_ring_buffer structure (V2)" and it's a regression from v2.6.34. > > Addresses: https://bugzilla.kernel.org/show_bug.cgi?id=16247 > > Signed-off-by: Dan Carpenter > Reported-by: Benny Halevy > Tested-by: Benny Halevy Reminds me, Gordon can you add module unload testing to your set of basic daily tests? To unload you need to unbind the fbcon interface first, my script is like this: echo 0 > /sys/class/vtconsole/vtcon1/bind rmmod i915 rmmod drm_kms_helper rmmod drm modprobe i915 echo 1 > /sys/class/vtconsole/vtcon1/bind If unload and re-load doesn't work please file a bug and see if you can bisect it. Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm: add vblank event trace point
Emit a trace point for vblank events. This can be helpful for mapping
drawing activity against the vblank frequency and period.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/Makefile |5 +++-
drivers/gpu/drm/drm_irq.c |3 ++
drivers/gpu/drm/drm_trace.h| 37
drivers/gpu/drm/drm_trace_points.c |4 +++
4 files changed, 48 insertions(+), 1 deletions(-)
create mode 100644 drivers/gpu/drm/drm_trace.h
create mode 100644 drivers/gpu/drm/drm_trace_points.c
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index abe3f44..3b02e04 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -11,7 +11,8 @@ drm-y :=drm_auth.o drm_buffer.o drm_bufs.o
drm_cache.o \
drm_agpsupport.o drm_scatter.o ati_pcigart.o drm_pci.o \
drm_sysfs.o drm_hashtab.o drm_sman.o drm_mm.o \
drm_crtc.o drm_modes.o drm_edid.o \
- drm_info.o drm_debugfs.o drm_encoder_slave.o
+ drm_info.o drm_debugfs.o drm_encoder_slave.o \
+ drm_trace_points.o
drm-$(CONFIG_COMPAT) += drm_ioc32.o
@@ -19,6 +20,8 @@ drm_kms_helper-y := drm_fb_helper.o drm_crtc_helper.o
drm_dp_i2c_helper.o
obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
+CFLAGS_drm_trace_points.o := -I$(src)
+
obj-$(CONFIG_DRM) += drm.o
obj-$(CONFIG_DRM_TTM) += ttm/
obj-$(CONFIG_DRM_TDFX) += tdfx/
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a263b70..6d201a8 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -34,6 +34,7 @@
*/
#include "drmP.h"
+#include "drm_trace.h"
#include/* For task queue support */
#include
@@ -754,6 +755,8 @@ void drm_handle_vblank_events(struct drm_device *dev, int
crtc)
}
spin_unlock_irqrestore(&dev->event_lock, flags);
+
+ trace_drm_vblank_event(crtc, seq);
}
/**
diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
new file mode 100644
index 000..8a92683
--- /dev/null
+++ b/drivers/gpu/drm/drm_trace.h
@@ -0,0 +1,37 @@
+#if !defined(_DRM_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define _DRM_TRACE_H_
+
+#include
+#include
+#include
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM drm
+#define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM)
+#define TRACE_INCLUDE_FILE drm_trace
+
+TRACE_EVENT(drm_vblank_event,
+
+ TP_PROTO(int crtc, unsigned int seq),
+
+ TP_ARGS(crtc, seq),
+
+ TP_STRUCT__entry(
+ __field(int, crtc)
+ __field(unsigned int, seq)
+ ),
+
+ TP_fast_assign(
+ __entry->crtc = crtc;
+ __entry->seq = seq;
+ ),
+
+ TP_printk("crtc=%d, seq=%d", __entry->crtc, __entry->seq)
+);
+
+#endif /* _DRM_TRACE_H_ */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#include
diff --git a/drivers/gpu/drm/drm_trace_points.c
b/drivers/gpu/drm/drm_trace_points.c
new file mode 100644
index 000..0d0eb90
--- /dev/null
+++ b/drivers/gpu/drm/drm_trace_points.c
@@ -0,0 +1,4 @@
+#include "drmP.h"
+
+#define CREATE_TRACE_POINTS
+#include "drm_trace.h"
--
1.6.1.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm: add per-event vblank event trace points
Allows us to track each process that requests and completes events.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_irq.c |8 ++
drivers/gpu/drm/drm_trace.h | 57 --
include/drm/drmP.h |2 +
3 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 6d201a8..c2ecb3e 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -588,6 +588,7 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
return -ENOMEM;
e->pipe = pipe;
+ e->base.pid = current->pid;
e->event.base.type = DRM_EVENT_VBLANK;
e->event.base.length = sizeof e->event;
e->event.user_data = vblwait->request.signal;
@@ -615,6 +616,9 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
DRM_DEBUG("event on vblank count %d, current %d, crtc %d\n",
vblwait->request.sequence, seq, pipe);
+ trace_drm_vblank_event_queued(current->pid, pipe,
+ vblwait->request.sequence);
+
e->event.sequence = vblwait->request.sequence;
if ((seq - vblwait->request.sequence) <= (1 << 23)) {
e->event.tv_sec = now.tv_sec;
@@ -622,6 +626,8 @@ static int drm_queue_vblank_event(struct drm_device *dev,
int pipe,
drm_vblank_put(dev, e->pipe);
list_add_tail(&e->base.link, &e->base.file_priv->event_list);
wake_up_interruptible(&e->base.file_priv->event_wait);
+ trace_drm_vblank_event_delivered(current->pid, pipe,
+vblwait->request.sequence);
} else {
list_add_tail(&e->base.link, &dev->vblank_event_list);
}
@@ -752,6 +758,8 @@ void drm_handle_vblank_events(struct drm_device *dev, int
crtc)
drm_vblank_put(dev, e->pipe);
list_move_tail(&e->base.link, &e->base.file_priv->event_list);
wake_up_interruptible(&e->base.file_priv->event_wait);
+ trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
+e->event.sequence);
}
spin_unlock_irqrestore(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/drm_trace.h b/drivers/gpu/drm/drm_trace.h
index 8a92683..03ea964 100644
--- a/drivers/gpu/drm/drm_trace.h
+++ b/drivers/gpu/drm/drm_trace.h
@@ -11,22 +11,51 @@
#define TRACE_INCLUDE_FILE drm_trace
TRACE_EVENT(drm_vblank_event,
+ TP_PROTO(int crtc, unsigned int seq),
+ TP_ARGS(crtc, seq),
+ TP_STRUCT__entry(
+ __field(int, crtc)
+ __field(unsigned int, seq)
+ ),
+ TP_fast_assign(
+ __entry->crtc = crtc;
+ __entry->seq = seq;
+ ),
+ TP_printk("crtc=%d, seq=%d", __entry->crtc, __entry->seq)
+);
- TP_PROTO(int crtc, unsigned int seq),
-
- TP_ARGS(crtc, seq),
-
- TP_STRUCT__entry(
- __field(int, crtc)
- __field(unsigned int, seq)
- ),
-
- TP_fast_assign(
- __entry->crtc = crtc;
- __entry->seq = seq;
- ),
+TRACE_EVENT(drm_vblank_event_queued,
+ TP_PROTO(pid_t pid, int crtc, unsigned int seq),
+ TP_ARGS(pid, crtc, seq),
+ TP_STRUCT__entry(
+ __field(pid_t, pid)
+ __field(int, crtc)
+ __field(unsigned int, seq)
+ ),
+ TP_fast_assign(
+ __entry->pid = pid;
+ __entry->crtc = crtc;
+ __entry->seq = seq;
+ ),
+ TP_printk("pid=%d, crtc=%d, seq=%d", __entry->pid, __entry->crtc, \
+ __entry->seq)
+);
- TP_printk("crtc=%d, seq=%d", __entry->crtc, __entry->seq)
+TRACE_EVENT(drm_vblank_event_delivered,
+ TP_PROTO(pid_t pid, int crtc, unsigned int seq),
+ TP_ARGS(pid, crtc, seq),
+ TP_STRUCT__entry(
+ __field(pid_t, pid)
+ __field(int, crtc)
+ __field(unsigned int, seq)
+ ),
+ TP_fast_assign(
+ __entry->pid = pid;
+ __entry->crtc = crtc;
+ __entry->seq = seq;
+ ),
+ TP_printk("pid=%d, crtc=%d, seq=%d", __entry->pid, __entry->crtc, \
+ __entry->seq)
);
#endif /* _DRM_TRACE_H_ */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c1b9871..8364a70 1
[PATCH 3/3] drm/i915: add tracepoints for flip requests & completions
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_trace.h| 36 ++
drivers/gpu/drm/i915/intel_display.c |5
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_trace.h
b/drivers/gpu/drm/i915/i915_trace.h
index fab2176..fea97a2 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -262,6 +262,42 @@ DEFINE_EVENT(i915_ring, i915_ring_wait_end,
TP_ARGS(dev)
);
+TRACE_EVENT(i915_flip_request,
+ TP_PROTO(int plane, struct drm_gem_object *obj),
+
+ TP_ARGS(plane, obj),
+
+ TP_STRUCT__entry(
+ __field(int, plane)
+ __field(struct drm_gem_object *, obj)
+ ),
+
+ TP_fast_assign(
+ __entry->plane = plane;
+ __entry->obj = obj;
+ ),
+
+ TP_printk("plane=%d, obj=%p", __entry->plane, __entry->obj)
+);
+
+TRACE_EVENT(i915_flip_complete,
+ TP_PROTO(int plane, struct drm_gem_object *obj),
+
+ TP_ARGS(plane, obj),
+
+ TP_STRUCT__entry(
+ __field(int, plane)
+ __field(struct drm_gem_object *, obj)
+ ),
+
+ TP_fast_assign(
+ __entry->plane = plane;
+ __entry->obj = obj;
+ ),
+
+ TP_printk("plane=%d, obj=%p", __entry->plane, __entry->obj)
+);
+
#endif /* _I915_TRACE_H_ */
/* This part must be outside protection */
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 19d2d3d..f7084b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -33,6 +33,7 @@
#include "intel_drv.h"
#include "i915_drm.h"
#include "i915_drv.h"
+#include "i915_trace.h"
#include "drm_dp_helper.h"
#include "drm_crtc_helper.h"
@@ -4648,6 +4649,8 @@ void intel_finish_page_flip(struct drm_device *dev, int
pipe)
atomic_dec_and_test(&obj_priv->pending_flip))
DRM_WAKEUP(&dev_priv->pending_flip_queue);
schedule_work(&work->work);
+
+ trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
}
void intel_prepare_page_flip(struct drm_device *dev, int plane)
@@ -4749,6 +4752,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
mutex_unlock(&dev->struct_mutex);
+ trace_i915_flip_request(intel_crtc->plane, obj);
+
return 0;
}
--
1.6.1.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: correctly update connector DPMS status in drm_fb_helper
We don't currently update the DPMS status of the connector (both in the
connector itself and the connector's DPMS property) in the fb helper
code. This means that if the kernel FB core has blanked the screen,
sysfs will still show a DPMS status of "on". It also means that when X
starts, it will try to light up the connectors, but the drm_crtc_helper
code will ignore the DPMS change since according to the connector, the
DPMS status is already on.
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=28436 (the annoying
"my screen was blanked when I started X and now it won't light up" bug).
Signed-off-by: Jesse Barnes
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b3779d2..32f67cb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -315,8 +315,9 @@ static void drm_fb_helper_on(struct fb_info *info)
struct drm_device *dev = fb_helper->dev;
struct drm_crtc *crtc;
struct drm_crtc_helper_funcs *crtc_funcs;
+ struct drm_connector *connector;
struct drm_encoder *encoder;
- int i;
+ int i, j;
/*
* For each CRTC in this fb, turn the crtc on then,
@@ -332,7 +333,14 @@ static void drm_fb_helper_on(struct fb_info *info)
crtc_funcs->dpms(crtc, DRM_MODE_DPMS_ON);
-
+ /* Walk the connectors & encoders on this fb turning them on */
+ for (j = 0; j < fb_helper->connector_count; j++) {
+ connector = fb_helper->connector_info[j]->connector;
+ connector->dpms = DRM_MODE_DPMS_ON;
+ drm_connector_property_set_value(connector,
+
dev->mode_config.dpms_property,
+DRM_MODE_DPMS_ON);
+ }
/* Found a CRTC on this fb, now find encoders */
list_for_each_entry(encoder, &dev->mode_config.encoder_list,
head) {
if (encoder->crtc == crtc) {
@@ -352,8 +360,9 @@ static void drm_fb_helper_off(struct fb_info *info, int
dpms_mode)
struct drm_device *dev = fb_helper->dev;
struct drm_crtc *crtc;
struct drm_crtc_helper_funcs *crtc_funcs;
+ struct drm_connector *connector;
struct drm_encoder *encoder;
- int i;
+ int i, j;
/*
* For each CRTC in this fb, find all associated encoders
@@ -367,6 +376,14 @@ static void drm_fb_helper_off(struct fb_info *info, int
dpms_mode)
if (!crtc->enabled)
continue;
+ /* Walk the connectors on this fb and mark them off */
+ for (j = 0; j < fb_helper->connector_count; j++) {
+ connector = fb_helper->connector_info[j]->connector;
+ connector->dpms = dpms_mode;
+ drm_connector_property_set_value(connector,
+
dev->mode_config.dpms_property,
+dpms_mode);
+ }
/* Found a CRTC on this fb, now find encoders */
list_for_each_entry(encoder, &dev->mode_config.encoder_list,
head) {
if (encoder->crtc == crtc) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Documentation of DRM's API?
On Sun, 4 Jul 2010 17:36:29 +0200
Rafał Miłecki wrote:
> Hi,
>
> While it's quite easy to track initialization of existing KMS drivers
> (plus we have http://www.botchco.com/agd5f/?p=50) I am not sure where
> to find documentation of API. I know driver has to register some
> outputs/encoders/transmitters related functions that will perform
> querying and setting. Is this possible to get some info about this?
> I'm sure there are some functions that are required and other that are
> optional (like DPMS vs. disable?).
>
> I saw there is something like Documentation/DocBook/drm.tmpl but can not use
> it:
>
> # make pdfdocs
> (...)
> (/usr/share/texmf/tex/latex/psnfss/t1phv.fd)
> (/usr/share/texmf/tex/latex/psnfss/t1pcr.fd)
> [1{/var/lib/texmf/fonts/map/pdftex
> /updmap/pdftex.map}] (/usr/share/texmf/tex/latex/psnfss/ts1ptm.fd)
> ! Missing number, treated as zero.
>
>p
> l.225 />
>
>
> ?
> ! Emergency stop.
>
>p
> l.225 />
>
>
> ! ==> Fatal error occurred, no output PDF file produced!
> Transcript written on tmp.log.
> make[1]: *** [Documentation/DocBook/z8530book.pdf] Błąd 1
> make: *** [pdfdocs] Błąd 2
>
> Is there any other place to look for docs?
You can edit the DocBook/Makefile to remove the broken docs from the
build. That should get you going with the drm doc itself. Much of the
drm source has doxygen rather than kdoc style comments though, I need
to clean those up before I can actually include the source generated
info in the drm.tmpl.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/gem: Add new flink_to ioctl
On Thu, 08 Jul 2010 17:37:20 +0100 Chris Wilson wrote: > On Thu, 8 Jul 2010 12:14:28 -0400, Kristian Høgsberg > wrote: > > On Thu, Jul 8, 2010 at 11:59 AM, Keith Packard wrote: > > > On Thu, 8 Jul 2010 11:23:25 -0400, Kristian Høgsberg > > > wrote: > > > > > >> - a mechanism to attach a binary blob to an flink_to buffer name. > > >> open_with_data returns the data. Userspace (typically libdrm) > > >> decides the layout and versioning of the blob and the contents > > >> will be chipset specific. it's an opaque blob to the kernel, > > >> which doesn't need to know about stride and formats etc. > > > > > > Arbitrary binary blobs considered harmful? Even if the kernel doesn't > > > need to know all of this data, having it in an explicit (versioned) > > > format will protect applications from randomly mis-interpreting the data. > > > > I talked with ickle about that and whether or not to include a > > version+format u32 for the data in the ioctl args. He convinced me > > that the kernel didn't need to know about the layout of the blob and > > that requiring by convention that the first u32 of the blob is the > > version+format u32 would suffice. I can go either way on this, but I > > guess I have a small preference for making it part of the ioctl args > > as you suggest. > > I am not going to argue with someone who has been tackling the issue of > protocol extensions for 25 years... ;-) > > My argument was based around that the current system is designed as a > directory of opaque objects and so the extended attributes should be > kept opaque to the kernel as well and left open to interpretation by > userland. What I am most unclear about is under which circumstances is > this backchannel communication preferable to passing the extra information > over the IPC that needs to be performed anyway in order to open a surface. That's the part I had trouble with as well. Passing the blob through the kernel saves a little IPC but also seems unnecessary, and so rubs against my kernel minimalist side... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: Return EBUSY if the framebuffer is unbound when flipping.
On Sat, 17 Jul 2010 20:23:26 +0100 Chris Wilson wrote: > It looks like there is a race condition between unbinding a > framebuffer on a hotplug event and user space trying to flip: > Nice, yeah that would definitely be a problem. Alternately we could solder everyone's configuration into something they couldn't change. :) Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm: allow drivers to provide their own EDID fetching routine
Make drm_edid_read take a new argument, edid_read, to allow drivers to
provide their own EDID fetch routine. Export the bit banging DDC over
i2c version of the EDID fetching routine and make the drivers use it.
This sets the stage for GMBUS support in the Intel driver.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_edid.c | 42 +++---
drivers/gpu/drm/i915/intel_drv.h|1 +
drivers/gpu/drm/i915/intel_hdmi.c |2 +-
drivers/gpu/drm/i915/intel_modes.c |3 +-
drivers/gpu/drm/i915/intel_sdvo.c |9 --
drivers/gpu/drm/nouveau/nouveau_connector.c |8 -
drivers/gpu/drm/radeon/radeon_connectors.c |5 ++-
drivers/gpu/drm/radeon/radeon_display.c |6 ++--
include/drm/drm_crtc.h |5 ++-
include/drm/drm_edid.h |3 ++
10 files changed, 47 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 83d8072..fb720e2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -214,10 +214,10 @@ EXPORT_SYMBOL(drm_edid_is_valid);
*
* Try to fetch EDID information by calling i2c driver function.
*/
-static int
-drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
- int block, int len)
+int
+drm_ddc_i2c_edid_read(void *data, unsigned char *buf, int block, int len)
{
+ struct i2c_adapter *adapter = data;
unsigned char start = block * EDID_LENGTH;
struct i2c_msg msgs[] = {
{
@@ -238,9 +238,13 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter,
unsigned char *buf,
return -1;
}
+EXPORT_SYMBOL(drm_ddc_i2c_edid_read);
static u8 *
-drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
+drm_do_get_edid(struct drm_connector *connector,
+ int (*edid_read)(void *data, unsigned char *buf, int block,
+int len),
+ void *data)
{
int i, j = 0;
u8 *block, *new;
@@ -250,7 +254,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
/* base block fetch */
for (i = 0; i < 4; i++) {
- if (drm_do_probe_ddc_edid(adapter, block, 0, EDID_LENGTH))
+ if (edid_read(data, block, 0, EDID_LENGTH))
goto out;
if (drm_edid_block_valid(block))
break;
@@ -269,8 +273,7 @@ drm_do_get_edid(struct drm_connector *connector, struct
i2c_adapter *adapter)
for (j = 1; j <= block[0x7e]; j++) {
for (i = 0; i < 4; i++) {
- if (drm_do_probe_ddc_edid(adapter, block, j,
- EDID_LENGTH))
+ if (edid_read(data, block, j, EDID_LENGTH))
goto out;
if (drm_edid_block_valid(block + j * EDID_LENGTH))
break;
@@ -291,20 +294,6 @@ out:
}
/**
- * Probe DDC presence.
- *
- * \param adapter : i2c device adaptor
- * \return 1 on success
- */
-static bool
-drm_probe_ddc(struct i2c_adapter *adapter)
-{
- unsigned char out;
-
- return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
-}
-
-/**
* drm_get_edid - get EDID data, if available
* @connector: connector we're probing
* @adapter: i2c adapter to use for DDC
@@ -315,12 +304,17 @@ drm_probe_ddc(struct i2c_adapter *adapter)
* Return edid data or NULL if we couldn't find any.
*/
struct edid *drm_get_edid(struct drm_connector *connector,
- struct i2c_adapter *adapter)
+ int (*edid_read)(void *data, unsigned char *buf,
+ int block, int len),
+ void *data)
{
struct edid *edid = NULL;
+ unsigned char out;
- if (drm_probe_ddc(adapter))
- edid = (struct edid *)drm_do_get_edid(connector, adapter);
+ /* Check for presence first */
+ if (edid_read(data, &out, 0, 1) == 0)
+ edid = (struct edid *)drm_do_get_edid(connector, edid_read,
+ data);
connector->display_info.raw_edid = (char *)edid;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3fbedd8..75c7161 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -167,6 +167,7 @@ int intel_ddc_get_modes(struct drm_connector *c, struct
i2c_adapter *adapter);
extern bool intel_ddc_probe(struct intel_encoder *intel_encoder);
void intel_i2c_quirk_set(struct drm_device *dev, bool enable);
void intel_i2c_reset_gmbus(struct drm_device *dev);
+int intel_gmbus_get_modes(struct drm_connector *c, int pin);
extern void intel_crt_init(struct drm_device *dev);
extern void intel_hd
[PATCH 2/2] drm/i915: use GMBUS for EDID fetching
Use the GMBUS interface rather than direct bit banging to grab the EDID
over DDC. The hope is that this method will be more reliable than bit
banging for fetching EDIDs from buggy monitors or through switches.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_reg.h| 52 ++--
drivers/gpu/drm/i915/intel_crt.c |7 ++
drivers/gpu/drm/i915/intel_modes.c | 164 +++-
3 files changed, 216 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 42c6024..b89599c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -507,12 +507,52 @@
# define GPIO_DATA_VAL_IN (1 << 12)
# define GPIO_DATA_PULLUP_DISABLE (1 << 13)
-#define GMBUS0 0x5100
-#define GMBUS1 0x5104
-#define GMBUS2 0x5108
-#define GMBUS3 0x510c
-#define GMBUS4 0x5110
-#define GMBUS5 0x5120
+#define GMBUS0 0x5100 /* clock/port select */
+#define GMBUS_RATE_100KHZ(0<<8)
+#define GMBUS_RATE_50KHZ (1<<8)
+#define GMBUS_RATE_400KHZ(2<<8) /* reserved on Pineview */
+#define GMBUS_RATE_1MHZ (3<<8) /* reserved on Pineview */
+#define GMBUS_HOLD_EXT (1<<7) /* 300ns hold time, rsvd on Pineview */
+#define GMBUS_PORT_DISABLED 0
+#define GMBUS_PORT_SSC 1
+#define GMBUS_PORT_VGADDC2
+#define GMBUS_PORT_PANEL 3
+#define GMBUS_PORT_DPC 4 /* HDMIC */
+#define GMBUS_PORT_DPB 5 /* SDVO, HDMIB */
+#define GMBUS_PORT_DPD 6 /* HDMID */
+ /* 7 reserved */
+#define GMBUS1 0x5104 /* command/status */
+#define GMBUS_SW_CLR_INT (1<<31)
+#define GMBUS_SW_RDY (1<<30)
+#define GMBUS_ENT(1<<29) /* enable timeout */
+#define GMBUS_CYCLE_NONE (0<<25)
+#define GMBUS_CYCLE_NI_NS_WAIT (1<<25)
+#define GMBUS_CYCLE_I_NS_WAIT(3<<25)
+#define GMBUS_CYCLE_STOP (4<<25)
+#define GMBUS_CYCLE_NI_STOP (5<<25)
+#define GMBUS_CYCLE_I_STOP (7<<25)
+#define GMBUS_BYTE_COUNT_SHIFT 16
+#define GMBUS_SLAVE_INDEX_SHIFT 8
+#define GMBUS_SLAVE_ADDR_SHIFT 1
+#define GMBUS_SLAVE_READ (1<<0)
+#define GMBUS_SLAVE_WRITE(0<<0)
+#define GMBUS2 0x5108 /* status */
+#define GMBUS_INUSE (1<<15)
+#define GMBUS_HW_WAIT_PHASE (1<<14)
+#define GMBUS_STALL_TIMEOUT (1<<13)
+#define GMBUS_INT(1<<12)
+#define GMBUS_HW_RDY (1<<11)
+#define GMBUS_SATOER (1<<10)
+#define GMBUS_ACTIVE (1<<9)
+#define GMBUS3 0x510c /* data buffer bytes 3-0 */
+#define GMBUS4 0x5110 /* interrupt mask (Pineview+) */
+#define GMBUS_SLAVE_TIMEOUT_EN (1<<4)
+#define GMBUS_NAK_EN (1<<3)
+#define GMBUS_IDLE_EN(1<<2)
+#define GMBUS_HW_WAIT_EN (1<<1)
+#define GMBUS_HW_RDY_EN (1<<0)
+#define GMBUS5 0x5120 /* byte index */
+#define GMBUS_2BYTE_INDEX_EN (1<<31)
/*
* Clock control & power management
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index ee0732b..60dc11b 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -453,6 +453,12 @@ static int intel_crt_get_modes(struct drm_connector
*connector)
struct i2c_adapter *ddc_bus;
struct drm_device *dev = connector->dev;
+ /* Try GMBUS first */
+ ret = intel_gmbus_get_modes(connector, 0);
+ if (ret) {
+ DRM_DEBUG_DRIVER("got EDID from GMBUS\n");
+ goto end;
+ }
ret = intel_ddc_get_modes(connector, intel_encoder->ddc_bus);
if (ret || !IS_G4X(dev))
@@ -466,6 +472,7 @@ static int intel_crt_get_modes(struct drm_connector
*connector)
"DDC bus registration failed for CRTDDC_D.\n");
goto end;
}
+
/* Try to get modes by GPIOD port */
ret = intel_ddc_get_modes(connector, ddc_bus);
intel_i2c_destroy(ddc_bus);
diff --git a/drivers/gpu/drm/i915/intel_modes.c
b/drivers/gpu/drm/i915/intel_modes.c
index 35d15f8..c9b946d 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2007 Dave Airlie
- * Copyright (c) 2007 Intel Corporation
+ * Copyright (c) 2007, 2010 Intel Corporation
* Jesse Barnes
*
* Permission is hereby granted, free of charge, to any person obtaining a
@@ -89,3 +89,165 @@ int intel_ddc_get_modes(struct drm_connector *connector,
return ret;
}
+
+static void intel_dump_gmbus(drm_i915_private_t *dev_priv)
+{
+ DRM_DEB
Re: [PATCH 1/2] drm: allow drivers to provide their own EDID fetching routine
On Wed, 21 Jul 2010 08:54:30 +1000 Dave Airlie wrote: > On Tue, 2010-07-20 at 15:44 -0700, Jesse Barnes wrote: > > Make drm_edid_read take a new argument, edid_read, to allow drivers to > > provide their own EDID fetch routine. Export the bit banging DDC over > > i2c version of the EDID fetching routine and make the drivers use it. > > This sets the stage for GMBUS support in the Intel driver. > > > > I think this needs some rework. > > You might want to checkout what the radeon driver does for hw i2c > engine. You should set up your own i2c hw handlers and use those instead > of bypassing the i2c stack. GMBUS is just another i2c hw block. I'll check it out, but I don't see what using the i2c stack buys us here except for obfuscation... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: allow drivers to provide their own EDID fetching routine
On Wed, 21 Jul 2010 09:27:54 +1000 Dave Airlie wrote: > On Tue, 2010-07-20 at 16:05 -0700, Jesse Barnes wrote: > > On Wed, 21 Jul 2010 08:54:30 +1000 > > Dave Airlie wrote: > > > > > On Tue, 2010-07-20 at 15:44 -0700, Jesse Barnes wrote: > > > > Make drm_edid_read take a new argument, edid_read, to allow drivers to > > > > provide their own EDID fetch routine. Export the bit banging DDC over > > > > i2c version of the EDID fetching routine and make the drivers use it. > > > > This sets the stage for GMBUS support in the Intel driver. > > > > > > > > > > I think this needs some rework. > > > > > > You might want to checkout what the radeon driver does for hw i2c > > > engine. You should set up your own i2c hw handlers and use those instead > > > of bypassing the i2c stack. GMBUS is just another i2c hw block. > > > > I'll check it out, but I don't see what using the i2c stack buys us > > here except for obfuscation... > > > > You'll want to use GMBUS for SDVO at some point in the future, or > something else, or you'll want to expose it to userspace for DDC/CI > users. Lots of reasons, its not obfuscation at all, what you are doing > is dodgy shortcuts. Using it for SDVO and other things means some other changes to the GMBUS code unfortunately. Still not seeing how using i2c makes userspace exposure or SDVO usage easier, but I don't care, I'll switch it around to use i2c core code. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm: allow drivers to provide their own EDID fetching routine
On Tue, 20 Jul 2010 16:34:39 -0700 Jesse Barnes wrote: > On Wed, 21 Jul 2010 09:27:54 +1000 > Dave Airlie wrote: > > > On Tue, 2010-07-20 at 16:05 -0700, Jesse Barnes wrote: > > > On Wed, 21 Jul 2010 08:54:30 +1000 > > > Dave Airlie wrote: > > > > > > > On Tue, 2010-07-20 at 15:44 -0700, Jesse Barnes wrote: > > > > > Make drm_edid_read take a new argument, edid_read, to allow drivers to > > > > > provide their own EDID fetch routine. Export the bit banging DDC over > > > > > i2c version of the EDID fetching routine and make the drivers use it. > > > > > This sets the stage for GMBUS support in the Intel driver. > > > > > > > > > > > > > I think this needs some rework. > > > > > > > > You might want to checkout what the radeon driver does for hw i2c > > > > engine. You should set up your own i2c hw handlers and use those instead > > > > of bypassing the i2c stack. GMBUS is just another i2c hw block. > > > > > > I'll check it out, but I don't see what using the i2c stack buys us > > > here except for obfuscation... > > > > > > > You'll want to use GMBUS for SDVO at some point in the future, or > > something else, or you'll want to expose it to userspace for DDC/CI > > users. Lots of reasons, its not obfuscation at all, what you are doing > > is dodgy shortcuts. > > Using it for SDVO and other things means some other changes to the > GMBUS code unfortunately. Still not seeing how using i2c makes > userspace exposure or SDVO usage easier, but I don't care, I'll switch > it around to use i2c core code. Assuming it helps a all of course. My hope here was that it would be able to get EDID out of configurations where we currently have trouble, like monitor switches or just plain crappy monitors. If it doesn't help I don't think there's much need to keep the code around... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: fixed brace, spacing and whitespace coding style issues
On Mon, 02 Aug 2010 10:16:53 +1000 Dave Airlie wrote: > On Wed, 2010-07-14 at 16:11 +0200, Nicolas Kaiser wrote: > > Fixed brace, spacing and whitespace coding style issues. > > Failed to apply to drm-core-next, might always be a pain to get > something like this to apply though, whitespace isn't top of my > priorities most days. I think you should just reject it, unless it comes as part of a series with actual work in it. At least that's been my policy lately... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/i915: Revert extra intel_wait_for_vblank to prevent stalls.
On Tue, 24 Aug 2010 16:56:16 +0100
Sitsofe Wheeler wrote:
> With the extra intel_wait_for_vblank added in commit
> 9d0498a2bf7455159b317f19531a3e5db2ecc9c4 periodic stalls were being
> triggered (which were detected by i915_hangcheck_elapsed). Partially
> revert this change for now.
>
> Signed-off-by: Sitsofe Wheeler
> ---
> drivers/gpu/drm/i915/intel_display.c |6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 116d938..534f1fa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2379,8 +2379,10 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int
> mode)
> I915_READ(dspbase_reg);
> }
>
> - /* Wait for vblank for the disable to take effect */
> - intel_wait_for_vblank_off(dev, pipe);
> + if (!IS_I9XX(dev)) {
> + /* Wait for vblank for the disable to take effect */
> + intel_wait_for_vblank_off(dev, pipe);
> + }
>
> /* Don't disable pipe A or pipe A PLLs if needed */
> if (pipeconf_reg == PIPEACONF &&
Hm why would we be triggering the hangcheck timer due to this code?
I'd rather figure that out and fix it before covering it up like this.
Wait for vblank off will spin until the display line reg stops
incrementing, so it's important that we flush any previous writes to
shut off the pipe before waiting. So adding a POSTING_READ() above it
might help? But that still doesn't explain why it would cause the
hangcheck timer to notice a hang...
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/i915: Revert wait for vblank to prevent X refresh issues
On Tue, 24 Aug 2010 16:53:59 +0100 Sitsofe Wheeler wrote: > In commit 9d0498a2bf7455159b317f19531a3e5db2ecc9c4 20ms waits were > converted into vblank waits. One of these caused tearing, mode detection > and redraw issues on an EeePC 900 with a more recent intel userspace ( > http://lkml.org/lkml/2010/8/23/432 ). Restoring the 20ms wait resolves > the issue. > > Signed-off-by: Sitsofe Wheeler > --- > drivers/gpu/drm/i915/intel_display.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 23157e1..116d938 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4539,7 +4539,7 @@ struct drm_crtc *intel_get_load_detect_pipe(struct > intel_encoder *intel_encoder, > encoder_funcs->commit(encoder); > } > /* let the connector get through one full cycle before testing */ > - intel_wait_for_vblank(dev, intel_crtc->pipe); > + msleep(20); > > return crtc; > } Wow, tearing, mode detection and redraw problems all because of this line? Maybe because we wait for a longer period here now? Can you check and see if we're timing out in the wait_for_vblank function? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm, video: fix use-before-NULL-check
On Fri, 27 Aug 2010 14:07:19 -0700
Kees Cook wrote:
> Fix potential crashes due to use-before-NULL situations.
>
> Signed-off-by: Kees Cook
> ---
> drivers/gpu/drm/drm_fb_helper.c |3 ++-
> drivers/media/video/em28xx/em28xx-video.c |3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index de82e20..8dd7e6f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -94,10 +94,11 @@ static bool
> drm_fb_helper_connector_parse_command_line(struct drm_fb_helper_conn
> int i;
> enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
> struct drm_fb_helper_cmdline_mode *cmdline_mode;
> - struct drm_connector *connector = fb_helper_conn->connector;
> + struct drm_connector *connector;
>
> if (!fb_helper_conn)
> return false;
> + connector = fb_helper_conn->connector;
>
> cmdline_mode = &fb_helper_conn->cmdline_mode;
> if (!mode_option)
> diff --git a/drivers/media/video/em28xx/em28xx-video.c
> b/drivers/media/video/em28xx/em28xx-video.c
> index 7b9ec6e..95a4b60 100644
> --- a/drivers/media/video/em28xx/em28xx-video.c
> +++ b/drivers/media/video/em28xx/em28xx-video.c
> @@ -277,12 +277,13 @@ static void em28xx_copy_vbi(struct em28xx *dev,
> {
> void *startwrite, *startread;
> int offset;
> - int bytesperline = dev->vbi_width;
> + int bytesperline;
>
> if (dev == NULL) {
> em28xx_isocdbg("dev is null\n");
> return;
> }
> + bytesperline = dev->vbi_width;
>
> if (dma_q == NULL) {
> em28xx_isocdbg("dma_q is null\n");
Look fine to me.
Reviewed-by: Jesse Barnes
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm: fix building on non-PCI platforms
On Wed, 8 Sep 2010 17:15:02 +0200 Arnd Bergmann wrote: > BenH's fix to correct building on multi-domain systems broke > building DRM for platforms without PCI support. This makes > the call to pci_domain_nr conditional in order to fix compilation. > > Signed-off-by: Arnd Bergmann > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 7809d23..5ff5819 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1099,7 +1099,11 @@ static inline int drm_get_pci_domain(struct drm_device > *dev) > return 0; > #endif /* __alpha__ */ > > +#ifdef CONFIG_PCI > return pci_domain_nr(dev->pdev->bus); > +#else > + return 0; > +#endif > } > > #if __OS_HAS_AGP I think we fixed this, but I guess Linus hasn't pulled my tree yet... -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Application calling DRI2SwapBuffers twice without updating buffers
On Mon, 13 Sep 2010 11:57:51 +0300 Pauli Nieminen wrote: > Hi, > > It is possible for client to call DRI2SwapBuffers twice in row even tough > client should update back buffer in between. Problem is exposed when > application does stupid thing like: > > renderScene(); > glXSwapBuffers(); > /* Might be long delay between but no rendering */ > glXSwapBuffers(); > > glx and egl specifications don't say anything if driver may fail in this case. > Only relevant part is that the back will hold undefined content. Second swap > will results front buffer holding undefined content. Which makes me think that > specification makes above code application error only that shouldn't be > detected. > > Is ddx driver allowed to fail the swap in this case? > > There is some special case when application doesn't have any back buffer. > Hardware limitation forces driver to share buffers between applications. Too > bad sharing causes that new application might takes over the back buffer > between swaps. > > The special case where there is no valid back for client because client > failed to render between swapbuffers has to fail either silently or with > error. > I would prefer to return error that is mostly likely ignored by application. > > Should driver fail all swaps that would result to the front buffer to have > completely undefined content? > > There could be simple detection for the application trying to show random > content. This would catch a few trivial application errors that would cause > corruption in screen. > > Detecting application errors (if it can be done cheaply) would help > application > developers understand that their code is causing problems. > > If driver is utilising invalidate events detection can't happen in server > side. We could print a warning in this case at the very least. Maybe if we try to swap again with the same dri2 invalidate stamp we could print an error. Kristian? Jesse -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use pipe state to tell when pipe is off
On Sun, 3 Oct 2010 00:33:06 -0700 Keith Packard wrote: > Instead of waiting for the display line value to settle, we can simply > wait for the pipe configuration register 'state' bit to turn off. > > Contrarywise, disabling the plane will not cause the display line > value to stop changing, so instead we wait for the vblank interrupt > bit to get set. And, we only do this when we're not about to wait for > the pipe to turn off. > > Signed-off-by: Keith Packard > --- Do these fixes help with the DP issues you've been seeing, Keith? Seems like the first one shouldn't change behavior since we ought to time out on waiting on vblank in that case, and the timeout is the same as the msleep we used to use. The second one looks like a good change, but really the pipe off change is separate from the plane disable bug fix. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use pipe state to tell when pipe is off
"Keith Packard" wrote: >On Sun, 3 Oct 2010 08:10:48 -0700, Jesse Barnes >wrote: > >> Do these fixes help with the DP issues you've been seeing, Keith? >> Seems like the first one shouldn't change behavior since we ought to >> time out on waiting on vblank in that case, and the timeout is the same >> as the msleep we used to use. > >The first one changes when the monitor sees the training message -- >before the change, the training message would get sent before waiting >for the vblank, and could potentially mess up the monitor >synchronization with signals. > >I tested this by turning an external DP monitor on/off repeatedly >without X running. Before the patch, the monitor would fail to sync once >in a while. With the patch, I haven't seen it fail. That's not to say >that it has actually fixed anything, just that it seems better. > >The best feature of the patch is that it shortens the time it takes to >light up a DP pipe -- the code was always hitting the timeout instead of >seeing a vblank signal, so we'd get a 50ms delay instead of a couple of ms. Yeah, definitely an improvement. I'll test myself when I get home (tested on my PCH eDP already, unfortunately didn't help). > >> The second one looks like a good change, but really the pipe off change >> is separate from the plane disable bug fix. > >Yeah, yeah, I know. I should have split the patch into two pieces... > >With these two patches in place, I'm not getting any timeouts while >waiting for vblank, which seems like a useful result, and should make >mode setting a tiny bit faster as well. > >I've got a couple more changes to work on today: > > 1) re-train the monitor when it gets unplugged and then plugged back >in. Right now, if you kick the cable out, you're stuck fumbling >around in the dark trying to run 'xrandr' again. Cool. Making use of our hotplug interrupts and never polling should be a goal; it looked like there were some other aux commands we could add as well. I'm meeting with the SV guys on Wed. To go over our eDP stuff, hopefully we can run the above through their equipment too and make sure we have all the timings etc correct. > > 2) send hotplug notification through the X server, at least for the >'reliable' hotplug signals. Right now, when you run 'xrandr' >after changing connections, gnome sees the connection status change >event and 'does stuff', which frequently collides with the 'xrandr' >command you're running. This is very confusing to users. Great, yeah we shouldn't be sending events in response to our normal status ioctls, that sounds broken. On 9xx and above we should be able to do reliable hotplug for everything (though at a power cost for TV and maybe VGA). -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use pipe state to tell when pipe is off
"Dave Airlie" wrote: >> mode setting a tiny bit faster as well. >> >> I've got a couple more changes to work on today: >> >> 1) re-train the monitor when it gets unplugged and then plugged back >> in. Right now, if you kick the cable out, you're stuck fumbling >> around in the dark trying to run 'xrandr' again. > >don't you do this already? both radeon/nouveau handle DP replug fine, >I thought Intel would have been where I stole the code from. There is some code to handle the interrupts, but I'm not sure if it's wired up. I think we rely on polling right now. -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm,kdb,kms: Add an enter argument to mode_set_base_atomic() API
On Tue, 12 Oct 2010 07:49:59 -0500 Jason Wessel wrote: > Some devices such as the pre nv02 chips have enter and exit > constraints where hardware compression must be turned off and > re-enabled on resuming normal operations. > > This patch extends the mode_set_base_atomic() call to pass an argument > to indicate if this is an entry or an exit from an atomic kernel mode > set change. Individual drm drivers can properly save and restore > state accordingly. > > Signed-off-by: Jason Wessel > CC: Jesse Barnes > CC: David Airlie > CC: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/drm_fb_helper.c |5 +++-- > drivers/gpu/drm/i915/intel_display.c|4 ++-- > drivers/gpu/drm/nouveau/nv04_crtc.c |2 +- > drivers/gpu/drm/nouveau/nv50_crtc.c |2 +- > drivers/gpu/drm/radeon/atombios_crtc.c |2 +- > drivers/gpu/drm/radeon/radeon_legacy_crtc.c |2 +- > drivers/gpu/drm/radeon/radeon_mode.h|4 ++-- > include/drm/drm_crtc_helper.h |3 ++- > 8 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 6a5e403..625a2d5 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -263,7 +263,8 @@ int drm_fb_helper_debug_enter(struct fb_info *info) > funcs->mode_set_base_atomic(mode_set->crtc, > mode_set->fb, > mode_set->x, > - mode_set->y); > + mode_set->y, > + 1); An enum for the last arg would make this call much less mysterious. :) -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm,kdb,kms: Add an enter argument to mode_set_base_atomic() API
On Tue, 12 Oct 2010 10:46:52 -0500
Jason Wessel wrote:
> On 10/12/2010 10:38 AM, Jesse Barnes wrote:
> > On Tue, 12 Oct 2010 07:49:59 -0500
> > Jason Wessel wrote:
> >
> >
> >> Some devices such as the pre nv02 chips have enter and exit
> >> constraints where hardware compression must be turned off and
> >> re-enabled on resuming normal operations.
> >>
> >> This patch extends the mode_set_base_atomic() call to pass an argument
> >> to indicate if this is an entry or an exit from an atomic kernel mode
> >> set change. Individual drm drivers can properly save and restore
> >> state accordingly.
> >>
> >> Signed-off-by: Jason Wessel
> >> CC: Jesse Barnes
> >> CC: David Airlie
> >> CC: dri-devel@lists.freedesktop.org
> >> ---
> >> drivers/gpu/drm/drm_fb_helper.c |5 +++--
> >> drivers/gpu/drm/i915/intel_display.c|4 ++--
> >> drivers/gpu/drm/nouveau/nv04_crtc.c |2 +-
> >> drivers/gpu/drm/nouveau/nv50_crtc.c |2 +-
> >> drivers/gpu/drm/radeon/atombios_crtc.c |2 +-
> >> drivers/gpu/drm/radeon/radeon_legacy_crtc.c |2 +-
> >> drivers/gpu/drm/radeon/radeon_mode.h|4 ++--
> >> include/drm/drm_crtc_helper.h |3 ++-
> >> 8 files changed, 13 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >> b/drivers/gpu/drm/drm_fb_helper.c
> >> index 6a5e403..625a2d5 100644
> >> --- a/drivers/gpu/drm/drm_fb_helper.c
> >> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >> @@ -263,7 +263,8 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
> >>funcs->mode_set_base_atomic(mode_set->crtc,
> >>mode_set->fb,
> >>mode_set->x,
> >> - mode_set->y);
> >> + mode_set->y,
> >> + 1);
> >>
> >
> > An enum for the last arg would make this call much less mysterious. :)
> >
> >
> Is there anything that is already predefined or would you prefer just
> true or false?
>
> This could be changed to a bool because there are really only two
> states, entering and exiting. If you provide some guidance on the
> preferred naming, I can re-spin the patch.
I was thinking:
enum mode_set_atomic {
ENTER_KDB,
EXIT_KDB,
};
or something similar (the name of the enum could probably be better).
Just having a number or true/false always makes me have to check the
function prototype. Having an enum means it's readable at the
callsite. But this is really just a nitpick; bool args like this are a
pet peeve of mine. :)
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, -next] drm/i915: blurred HDMI output
On Sun, 17 Oct 2010 19:37:13 +0200
Arnd Bergmann wrote:
> Commit e9e331a8ab "drm/i915/lvds: Ensure panel is unlocked for Ironlake or
> the panel fitter" broke my HDMI output on GMA4500HD, it seemed to be driving
> the output with the wrong resolution, while still showing the native panel
> resolution of 1680x1050 being in use. This caused a very irritating blur
> on the screen.
>
> After bisecting the problem, I managed to do a partial revert which fixes
> the problem and applies on top of linux-next. I have no idea what this patch
> actually does or if it can break any other configuration, but it solves the
> problem for me.
>
> Signed-off-by: Arnd Bergmann
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 9109c00..2d47161 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3921,6 +3921,10 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
> pipeconf |= PIPECONF_ENABLE;
> dpll |= DPLL_VCO_ENABLE;
>
> + /* Disable the panel fitter if it was on our pipe */
> + if (!HAS_PCH_SPLIT(dev) && intel_panel_fitter_pipe(dev) == pipe)
> + I915_WRITE(PFIT_CONTROL, 0);
> +
> DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe == 0 ? 'A' : 'B');
> drm_mode_debug_printmodeline(mode);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 40e99bf..de21a82 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -224,6 +224,7 @@ extern u32 intel_panel_get_max_backlight(struct
> drm_device *dev);
> extern u32 intel_panel_get_backlight(struct drm_device *dev);
> extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
>
> +extern int intel_panel_fitter_pipe (struct drm_device *dev);
> extern void intel_crtc_load_lut(struct drm_crtc *crtc);
> extern void intel_encoder_prepare (struct drm_encoder *encoder);
> extern void intel_encoder_commit (struct drm_encoder *encoder);
> diff --git a/drivers/gpu/drm/i915/intel_overlay.c
> b/drivers/gpu/drm/i915/intel_overlay.c
> index 375316a..da65d95 100644
> --- a/drivers/gpu/drm/i915/intel_overlay.c
> +++ b/drivers/gpu/drm/i915/intel_overlay.c
> @@ -1053,7 +1053,7 @@ static int check_overlay_src(struct drm_device *dev,
> * Return the pipe currently connected to the panel fitter,
> * or -1 if the panel fitter is not present or not in use
> */
> -static int intel_panel_fitter_pipe(struct drm_device *dev)
> +int intel_panel_fitter_pipe(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 pfit_control;
>
Hm, the LVDS code should take care of panel fitting in
the !HAS_PCH_SPLIT case. Does this patch achieve the same thing as
yours? Maybe we were leaving a stale PFIT value in place...
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds
index f1a6499..bc1e1c1 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1000,6 +1000,8 @@ void intel_lvds_init(struct drm_device *dev)
if (!intel_lvds->fixed_mode)
goto failed;
+ intel_lvds->pfit_dirty = true;
+
out:
if (HAS_PCH_SPLIT(dev)) {
u32 pwm;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, -next] drm/i915: blurred HDMI output
On Mon, 18 Oct 2010 21:13:59 +0200 Arnd Bergmann wrote: > On Monday 18 October 2010 20:56:48 Jesse Barnes wrote: > > Hm, the LVDS code should take care of panel fitting in > > the !HAS_PCH_SPLIT case. Does this patch achieve the same thing as > > yours? Maybe we were leaving a stale PFIT value in place... > > No, your patch doesn't have any effect for me. Oh right, you were complaining about HMDI. :) Yes we still need to shut off panel fitting elsewhere. Do you see the blurriness at boot or just after turning on HDMI while leaving the laptop panel enabled? -- Jesse Barnes, Intel Open Source Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, -next] drm/i915: blurred HDMI output
On Mon, 18 Oct 2010 21:57:05 +0200
Arnd Bergmann wrote:
> On Monday 18 October 2010 21:28:01 Jesse Barnes wrote:
> >
> > On Mon, 18 Oct 2010 21:13:59 +0200
> > Arnd Bergmann wrote:
> >
> > > On Monday 18 October 2010 20:56:48 Jesse Barnes wrote:
> > > > Hm, the LVDS code should take care of panel fitting in
> > > > the !HAS_PCH_SPLIT case. Does this patch achieve the same thing as
> > > > yours? Maybe we were leaving a stale PFIT value in place...
> > >
> > > No, your patch doesn't have any effect for me.
> >
> > Oh right, you were complaining about HMDI. :)
> >
> > Yes we still need to shut off panel fitting elsewhere. Do you see the
> > blurriness at boot or just after turning on HDMI while leaving the
> > laptop panel enabled?
>
> It's a G45 desktop machine, not a laptop.
>
> i915 is a module with kms enabled, the blur seems to show up on the
> text console the moment that the module gets loaded during boot.
> Before that, the console is in VGA text mode, which is always
> slightly blurred because the screen has a different native resolution.
Arg why would the BIOS set the panel fitting bit at all in this
config? Any attached outputs (VGA, HDMI) will have their own scalers...
Obviously we need to clear it somewhere, but we should be able to avoid
clearing it on every mode set. We should probably shut down all panel
related bits in intel_lvds.c if we fail to detect an LVDS output; that
should save a little power and avoid problems like this.
Maybe the below patch which does that will help?
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds
index f1a6499..da31963 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1024,6 +1024,9 @@ out:
failed:
DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
+ intel_lvds_set_power(intel_lvds, false);
+ I915_WRITE(PFIT_CONTROL, 0);
+ I915_WRITE(LVDS, 0);
drm_connector_cleanup(connector);
drm_encoder_cleanup(encoder);
kfree(intel_lvds);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, -next] drm/i915: blurred HDMI output
On Mon, 18 Oct 2010 22:37:43 +0200
Arnd Bergmann wrote:
> On Monday 18 October 2010 22:30:17 Arnd Bergmann wrote:
>
> > I don't think the code path you patch here actually gets used, since
> > intel_lvds_init gets called by intel_setup_outputs only for mobile devices.
>
> FWIW, the patch below does work.
>
> Arnd
>
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5311,6 +5311,9 @@ static void intel_setup_outputs(struct drm_device *dev)
> if (IS_MOBILE(dev) && !IS_I830(dev))
> intel_lvds_init(dev);
>
> + I915_WRITE(PFIT_CONTROL, 0);
> + I915_WRITE(LVDS, 0);
> +
> if (HAS_PCH_SPLIT(dev)) {
> dpd_is_edp = intel_dpd_is_edp(dev);
Oh of course, I was thinking the output functions did detection, but we
short circuit it before that.
So we should probably do it in setup_outputs or init_display once we've
figured out there's no LVDS. It's cool that the panel fitter still has
an effect even on non-LVDS platforms though, maybe we really should
treat it as a part of the CRTC rather than the LVDS encoder after all.
--
Jesse Barnes, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 01/21] drm/i915: Enable digital port hotplug on PCH systems
On Fri, 30 Sep 2011 18:17:32 +0200 Daniel Vetter wrote: > On Thu, Sep 29, 2011 at 06:09:33PM -0700, Keith Packard wrote: > > We were relying on the BIOS to set these bits, which doesn't always > > happen. > > > > Signed-off-by: Keith Packard > > Ok, I'll try to increase our review-bandwidth for modesetting stuff by > jumping into this maze myself. For the moment I'll shortly explain what > I've actually checked so you know how much weight/little to attach to my > r-b's. > > Checked with docs, noticed that the public one's lack register addresses > for PCH regs ... > Reviewed-by: Daniel Vetter Yep, I like it too: Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 04/21] drm/i915: Only use VBT panel mode on eDP if no EDID is found
On Fri, 30 Sep 2011 10:58:26 -0700 Keith Packard wrote: > On Fri, 30 Sep 2011 18:32:35 +0200, Daniel Vetter wrote: > > > Ok, this is way over my head, just checked whether the patch does what it > > claims to - nice exercise in reading dp modeset code ;-) > > Yeah, it's purely heuristic -- the VBT contains a mode which was > originally for the LVDS. It's unclear whether it should ever be applied > to an eDP panel, but absent other information, it seems like we should > at least consider it. > > Many LVDS panels don't bother to include an EDID rom, or the vendor > didn't bother to hook up the DDC wire; presumably it's cheaper for them > to stick more data in the VBIOS than add hardware. However, there are > some LVDS panels with EDID roms which contain *incorrect* mode data for > the panel (amazing, I know), and so the driver prefers to use the VBT > data when both are present. > > eDP, on the other hand, doesn't require any additional wiring (at least) > to connect up the DDC channel, and eDP panels are required to provide > EDID data. So far, in my (very small) sample set, I've got one machine > which provides accurate VBT *and* EDID data (an HP 2540p) and one > machine which provides inaccurate VBT data but accurate EDID data (a > MacBook air). So, I just decided to prefer the EDID data. > > One option would be to extract the current mode from the hardware when > the driver starts and use that if present. But, that might mean that > you'd get different modes depending on whether the machine booted with > the lid closed or open, which seems like a bad plan. More than that, I think eDP *requires* an EDID, and it sounds like even the Air has one (and if any machine didn't, you know it would be an Apple). So I'm definitely in favor of this change. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 05/21] drm/i915: Check eDP power when doing aux channel communications
On Thu, 29 Sep 2011 18:09:37 -0700
Keith Packard wrote:
> Verify that the eDP VDD is on, either with the panel being on or with
> the VDD force-on bit being set.
>
> This demonstrates that in many instances, VDD is not on when needed,
> which leads to failed EDID communications.
>
> Signed-off-by: Keith Packard
> ---
> drivers/gpu/drm/i915/intel_dp.c | 22 ++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8ab2a88..3b29a6f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -279,6 +279,24 @@ intel_hrawclk(struct drm_device *dev)
> }
> }
>
> +static void
> +intel_dp_check_edp(struct intel_dp *intel_dp)
> +{
> + struct drm_device *dev = intel_dp->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 pp_status, pp_control;
> + if (!is_edp(intel_dp))
> + return;
> + pp_status = I915_READ(PCH_PP_STATUS);
> + pp_control = I915_READ(PCH_PP_CONTROL);
> + if ((pp_status & PP_ON) == 0 && (pp_control & EDP_FORCE_VDD) == 0) {
> + WARN(1, "eDP powered off while attempting aux channel
> communication.\n");
> + DRM_DEBUG_KMS("Status 0x%08x Control 0x%08x\n",
> + pp_status,
> + I915_READ(PCH_PP_CONTROL));
> + }
> +}
I'd call it "intel_dp_assert_dp_power" or something more descriptive
(or just assert_panel_power to match the other asserts in
intel_display.c), but other than that this is a nice sanity check.
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 16/21] drm/i915: edp_panel_on does not need to return a bool
On Thu, 29 Sep 2011 18:09:48 -0700
Keith Packard wrote:
> The return value was unused, so just stop doing that.
>
> Signed-off-by: Keith Packard
> ---
> drivers/gpu/drm/i915/intel_dp.c |6 ++
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 02b5162..56146a2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -886,14 +886,14 @@ static void ironlake_edp_panel_vdd_off(struct intel_dp
> *intel_dp)
> }
>
> /* Returns true if the panel was already on when called */
> -static bool ironlake_edp_panel_on (struct intel_dp *intel_dp)
> +static void ironlake_edp_panel_on (struct intel_dp *intel_dp)
> {
Remove the comment too?
--
Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 10/21] drm/i915: Wrap DP EDID fetch functions to enable eDP panel power
On Thu, 29 Sep 2011 18:09:42 -0700 Keith Packard wrote: > Talking to the eDP DDC channel requires that the panel be powered > up. Wrap both the EDID and modes fetch code with calls to turn the vdd > power on and back off. > These VDD AUX changes look good, ack on all of them. -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 00/24] MacBook Air patch sequence (v2)
On Fri, 30 Sep 2011 11:17:38 -0700 Keith Packard wrote: > On Fri, 30 Sep 2011 09:20:55 -0400, Ted Ts'o wrote: > > > What kind of battery life do you get with these patches applied, out > > of curiosity? > > 4-5 hours when doing the usual web-surfing, etc. Seems pretty much the > same as people are getting under Mac OS. As long as you enable RC6. :) -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] [PATCH 9/9] drm/i915: Initialize PCH refclks at modeset init time
On Tue, 27 Sep 2011 11:11:57 -0700 Keith Packard wrote: > On Tue, 27 Sep 2011 17:56:39 +0100, Chris Wilson chris-wilson.co.uk> wrote: > > > Ah, now I see why we moved from using the active configuration earlier. ;-) > > My evil plan is revealed! > > > Doesn't this prevent us from ever using SSC though, as virtually every > > single PCH machine has HDMI encoders that haven't been masked out through > > the chicken fuses or VBT? > > That wasn't my intent -- the SSC source gets modulated whenever the VBT > table says it can, so when the panel uses the SSC source, it will get > SSC. Did I mess something up here? Or is it just some interaction with > the mode setting code that I didn't get right? Assuming we're selecting the proper reference clock in the PLL selection code anyway... Doing it all up front seems nicer; did you get confirmation that the "wavy VGA" bug was fixed with this series? Overall seems like a good improvement over our old PCH refclk code... -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] PCH reference clock cleanups
On Wed, 28 Sep 2011 15:22:48 -0300 Paulo Zanoni wrote: > 2011/9/27 Keith Packard : > > Here's a patch sequence which cleans up a bunch of PCH refclk related > > bits. > > For the series: Tested-by: Paulo Zanoni > > Tested all the patches on Ironlake (LVDS + VGA). Fixes fd.o bug #38750 for me. > > I also tested the patch you sent today 1 hour ago (inline in one of > the emails) and things still work with it. I'll keep using these > patches since they fix my laptop. Any problem will be reported. > > Maybe my email client/server is ruining things, but I believe patch 7 > includes whitespace errors. Yay excellent. Now... is keeping the various refclks enabled costing us any power? IOW, should we be trying to disable them when everything has been DPMS'd off too? -- Jesse Barnes, Intel Open Source Technology Center
[Intel-gfx] PCH reference clock cleanups
On Mon, 03 Oct 2011 16:18:48 -0700 Keith Packard wrote: > On Mon, 3 Oct 2011 14:14:23 -0700, Jesse Barnes > wrote: > > > Now... is keeping the various refclks enabled costing us any power? > > IOW, should we be trying to disable them when everything has been > > DPMS'd off too? > > That's the same as tracking usage and enabling/disabling on the fly as > modes are set. I think it's possible, but I'd like to have the 'simpler' > fix present before we try a power saving move. Agreed; fortunately shutting everything off when no outputs are active should be simpler than trying flip the bits on & off every mode set. :) -- Jesse Barnes, Intel Open Source Technology Center
[PATCH v2 4/5] DRI2: Expose API to set drawable swap limit.
On Thu, 15 Sep 2011 01:31:00 +0200 Mario Kleiner wrote: > On Sep 15, 2011, at 12:54 AM, Francisco Jerez wrote: > > > Mario Kleiner writes: > > > >> On Sep 14, 2011, at 6:02 PM, Keith Packard wrote: > >> > >>> On Wed, 14 Sep 2011 10:05:29 -0500, Jesse Barnes > >>> wrote: > >>> > >>>> Ah thanks Mario, I blame Keith. :p I agree we should integrate > >>>> this > >>>> patch as it would allow us to do more fun stuff with swapping... > >>> > >>> That patch changes a ton of stuff; would be nice to see it split > >>> into > >>> smaller chunks that could be reviewed easily. > >>> > >> > >> For reference, we're talking about this series, right? > >> > >> <http://www.mail-archive.com/xorg-devel at lists.x.org/msg14336.html> > >> > >> As far as i can see, they were already split up in chunks and > >> reviewed > >> by Jesse, me and Franzisco Jerez - assuming they still apply to the > >> latest server version? At least 1/5, 4/5 and 5/5 looked simple > >> enough > >> and 4/5 and 5/5, the swaplimit api, seem to be independent from the > >> rest of the series? > >> > > Note that my r-b only goes to 4/5, I had some objections to 5/5 and > > I'm > > not sure if 1/5-3/5 still make sense. > > I haven't checked if the rest still makes sense. 5/5 was about why a > driver who requests a dri2 swaplimit change should be called back to > confirm it is ok with the swaplimit change, which you said seems > totally redundant, right? > > Ok, so we talk specifically about 4/5, which was reviewed by all of > us, i think non-controversial, and a simple addition of a dri2 > swaplimit api. > > Keith, what about that one for a start? > > > BTW, is there any reason this is being discussed outside of the > > mailing > > list? > > No. It just started as a private conversation with Jesse and > "drifted" into this. cc'ing dri-devel, all that was said is in this > mail. What's the latest here? I still think we need the swap limit API... -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20111006/dc706677/attachment-0001.pgp>
DRM planes and new fb creation ioctl
I've given up waiting for someone to implement support for these ioctls on another platform before they're merged, but I have received a lot of feedback on the interfaces, and it sounds like they're ok. I've also fixed all the remaining issues I'm aware of on SNB platforms and things are working well, so I'm just going to push them out. (Note IVB support is still missing a few bits for scaling and such; I'll fix those up when I get back home and can test on IVB again.) One change you may notice from the last set is that I've removed the 'zpos' parameter. Plane blending and z ordering is very chipset specific (it even varies between Intel chipsets), so exposing it through a device specific ioctl is probably a better plan. By default, planes should just overlay the primary plane; a device specific ioctl (none available yet, but I have some planned for i915) can provide more flexibility. To recap previous posts, this patchset provides a few new interfaces: - addfb2 - a new FB creation ioctl that lets you specify a surface format, as defined by a fourcc code from the video4linux headers (libdrm will wrap these in DRM_ macros for portability) - planes - ioctls for fetching plane info and attaching an fb to a plane; note there's no separate flip ioctl for planes, just use setplane to update the fb The testdisplay.c program in intel-gpu-tools has support for testing these interfaces, and I'll be fixing up and pushing the xf86-video-intel support soon as well, so you can use either as a reference for how the new interfaces work. Thanks, Jesse
[PATCH 01/11] drm: add plane support
Planes are a bit like half-CRTCs. They have a location and fb, but
don't drive outputs directly. Add support for handling them to the core
KMS code.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_crtc.c | 236 +++-
drivers/gpu/drm/drm_drv.c |3 +
include/drm/drm.h |3 +
include/drm/drm_crtc.h | 71 +-
include/drm/drm_mode.h | 33 ++
5 files changed, 343 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fe738f0..0e129b1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -535,6 +535,48 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
}
EXPORT_SYMBOL(drm_encoder_cleanup);
+void drm_plane_init(struct drm_device *dev, struct drm_plane *plane,
+ unsigned long possible_crtcs,
+ const struct drm_plane_funcs *funcs,
+ uint32_t *formats, uint32_t format_count)
+{
+ mutex_lock(&dev->mode_config.mutex);
+
+ plane->dev = dev;
+ drm_mode_object_get(dev, &plane->base, DRM_MODE_OBJECT_PLANE);
+ plane->funcs = funcs;
+ plane->format_types = kmalloc(sizeof(uint32_t) * format_count,
+ GFP_KERNEL);
+ if (!plane->format_types) {
+ DRM_DEBUG_KMS("out of memory when allocating plane\n");
+ drm_mode_object_put(dev, &plane->base);
+ return;
+ }
+
+ memcpy(plane->format_types, formats, format_count);
+ plane->format_count = format_count;
+ plane->possible_crtcs = possible_crtcs;
+
+ list_add_tail(&plane->head, &dev->mode_config.plane_list);
+ dev->mode_config.num_plane++;
+
+ mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_plane_init);
+
+void drm_plane_cleanup(struct drm_plane *plane)
+{
+ struct drm_device *dev = plane->dev;
+
+ mutex_lock(&dev->mode_config.mutex);
+ kfree(plane->format_types);
+ drm_mode_object_put(dev, &plane->base);
+ list_del(&plane->head);
+ dev->mode_config.num_plane--;
+ mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_plane_cleanup);
+
/**
* drm_mode_create - create a new display mode
* @dev: DRM device
@@ -866,6 +908,7 @@ void drm_mode_config_init(struct drm_device *dev)
INIT_LIST_HEAD(&dev->mode_config.encoder_list);
INIT_LIST_HEAD(&dev->mode_config.property_list);
INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
+ INIT_LIST_HEAD(&dev->mode_config.plane_list);
idr_init(&dev->mode_config.crtc_idr);
mutex_lock(&dev->mode_config.mutex);
@@ -1466,6 +1509,193 @@ out:
}
/**
+ * drm_mode_getplane_res - get plane info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return an plane count and set of IDs.
+ */
+int drm_mode_getplane_res(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_plane_res *plane_resp = data;
+ struct drm_mode_config *config;
+ struct drm_plane *plane;
+ uint32_t __user *plane_ptr;
+ int copied = 0, ret = 0;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EINVAL;
+
+ mutex_lock(&dev->mode_config.mutex);
+ config = &dev->mode_config;
+
+ /*
+* This ioctl is called twice, once to determine how much space is
+* needed, and the 2nd time to fill it.
+*/
+ if (config->num_plane &&
+ (plane_resp->count_planes >= config->num_plane)) {
+ plane_ptr = (uint32_t *)(unsigned long)plane_resp->plane_id_ptr;
+
+ list_for_each_entry(plane, &config->plane_list, head) {
+ if (put_user(plane->base.id, plane_ptr + copied)) {
+ ret = -EFAULT;
+ goto out;
+ }
+ copied++;
+ }
+ }
+ plane_resp->count_planes = config->num_plane;
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+/**
+ * drm_mode_getplane - get plane info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return plane info, including formats supported, gamma size, any
+ * current fb, etc.
+ */
+int drm_mode_getplane(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_plane *plane_resp = data;
+ struct drm_mode_object *obj;
+ struct drm_plane *plane;
+ uint32_t __user *format_ptr;
+ int ret = 0;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ ret
[PATCH 02/11] drm: add an fb creation ioctl that takes a pixel format
To properly support the various plane formats supported by different
hardware, the kernel must know the pixel format of a framebuffer object.
So add a new ioctl taking a format argument corresponding to a fourcc
name from videodev2.h.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/drm_crtc.c| 71 -
drivers/gpu/drm/drm_crtc_helper.c |3 +-
drivers/gpu/drm/drm_drv.c |1 +
drivers/gpu/drm/i915/intel_display.c |9 ++--
drivers/gpu/drm/i915/intel_drv.h |2 +-
drivers/gpu/drm/i915/intel_fb.c |3 +-
drivers/gpu/drm/nouveau/nouveau_display.c |4 +-
drivers/gpu/drm/radeon/radeon_display.c |4 +-
drivers/gpu/drm/radeon/radeon_fb.c|5 +-
drivers/gpu/drm/radeon/radeon_mode.h |2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |2 +-
drivers/staging/gma500/framebuffer.c |2 +-
include/drm/drm.h |1 +
include/drm/drm_crtc.h|6 ++-
include/drm/drm_crtc_helper.h |2 +-
include/drm/drm_mode.h| 14 ++
16 files changed, 112 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 0e129b1..a30b9d4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1909,7 +1909,76 @@ out:
int drm_mode_addfb(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
- struct drm_mode_fb_cmd *r = data;
+ struct drm_mode_fb_cmd *or = data;
+ struct drm_mode_fb_cmd2 r;
+ struct drm_mode_config *config = &dev->mode_config;
+ struct drm_framebuffer *fb;
+ int ret = 0;
+
+ /* Use new struct with format internally */
+ r.fb_id = or->fb_id;
+ r.width = or->width;
+ r.height = or->height;
+ r.pitch = or->pitch;
+ r.bpp = or->bpp;
+ r.depth = or->depth;
+ r.pixel_format = 0;
+ r.handle = or->handle;
+
+ if (!drm_core_check_feature(dev, DRIVER_MODESET))
+ return -EINVAL;
+
+ if ((config->min_width > r.width) || (r.width > config->max_width)) {
+ DRM_ERROR("mode new framebuffer width not within limits\n");
+ return -EINVAL;
+ }
+ if ((config->min_height > r.height) || (r.height > config->max_height))
{
+ DRM_ERROR("mode new framebuffer height not within limits\n");
+ return -EINVAL;
+ }
+
+ mutex_lock(&dev->mode_config.mutex);
+
+ /* TODO check buffer is sufficiently large */
+ /* TODO setup destructor callback */
+
+ fb = dev->mode_config.funcs->fb_create(dev, file_priv, &r);
+ if (IS_ERR(fb)) {
+ DRM_ERROR("could not create framebuffer\n");
+ ret = PTR_ERR(fb);
+ goto out;
+ }
+
+ or->fb_id = fb->base.id;
+ list_add(&fb->filp_head, &file_priv->fbs);
+ DRM_DEBUG_KMS("[FB:%d]\n", fb->base.id);
+
+out:
+ mutex_unlock(&dev->mode_config.mutex);
+ return ret;
+}
+
+/**
+ * drm_mode_addfb2 - add an FB to the graphics configuration
+ * @inode: inode from the ioctl
+ * @filp: file * from the ioctl
+ * @cmd: cmd from ioctl
+ * @arg: arg from ioctl
+ *
+ * LOCKING:
+ * Takes mode config lock.
+ *
+ * Add a new FB to the specified CRTC, given a user request with format.
+ *
+ * Called by the user via ioctl.
+ *
+ * RETURNS:
+ * Zero on success, errno on failure.
+ */
+int drm_mode_addfb2(struct drm_device *dev,
+ void *data, struct drm_file *file_priv)
+{
+ struct drm_mode_fb_cmd2 *r = data;
struct drm_mode_config *config = &dev->mode_config;
struct drm_framebuffer *fb;
int ret = 0;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index f88a9b2..77c7293 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -806,13 +806,14 @@ void drm_helper_connector_dpms(struct drm_connector
*connector, int mode)
EXPORT_SYMBOL(drm_helper_connector_dpms);
int drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
- struct drm_mode_fb_cmd *mode_cmd)
+ struct drm_mode_fb_cmd2 *mode_cmd)
{
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
fb->pitch = mode_cmd->pitch;
fb->bits_per_pixel = mode_cmd->bpp;
fb->depth = mode_cmd->depth;
+ fb->pixel_format = mode_cmd->pixel_format;
return 0;
}
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 15da618..f24b9b6 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -152,6 +152,7 @@ static struct drm_ioctl_desc drm_ioctls[]
[PATCH 05/11] drm/i915: move pin & fence for plane past potential error paths
This avoids the need to unpin on the error path. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 2e38b15..e583bd0 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -62,9 +62,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, old_obj = intel_plane->obj; mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); - if (ret) - goto out_unlock; dvscntr = I915_READ(reg); @@ -104,6 +101,10 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, goto out_unlock; } + ret = intel_pin_and_fence_fb_obj(dev, obj, NULL); + if (ret) + goto out_unlock; + intel_plane->obj = obj; dvscntr |= DVS_TILED; -- 1.7.4.1
[PATCH 03/11] drm/i915: rename existing overlay support to "legacy"
The old overlay block has all sorts of quirks and is very different than
ILK+ video sprites. So rename it to legacy to make that clear and clash
less with core overlay support.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_debugfs.c |2 +-
drivers/gpu/drm/i915/i915_drv.h | 12 ++--
drivers/gpu/drm/i915/i915_irq.c |2 +-
drivers/gpu/drm/i915/intel_display.c |2 +-
drivers/gpu/drm/i915/intel_drv.h |4 +-
drivers/gpu/drm/i915/intel_overlay.c | 126 +-
6 files changed, 74 insertions(+), 74 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 8e95d66..b6d0bbc 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -825,7 +825,7 @@ static int i915_error_state(struct seq_file *m, void
*unused)
}
if (error->overlay)
- intel_overlay_print_error_state(m, error->overlay);
+ intel_legacy_overlay_print_error_state(m, error->overlay);
if (error->display)
intel_display_print_error_state(m, dev, error->display);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 06a37f4..b96c174 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -118,8 +118,8 @@ struct intel_opregion {
};
#define OPREGION_SIZE(8*1024)
-struct intel_overlay;
-struct intel_overlay_error_state;
+struct intel_legacy_overlay;
+struct intel_legacy_overlay_error_state;
struct drm_i915_master_private {
drm_local_map_t *sarea;
@@ -191,7 +191,7 @@ struct drm_i915_error_state {
u32 cache_level:2;
} *active_bo, *pinned_bo;
u32 active_bo_count, pinned_bo_count;
- struct intel_overlay_error_state *overlay;
+ struct intel_legacy_overlay_error_state *overlay;
struct intel_display_error_state *display;
};
@@ -343,7 +343,7 @@ typedef struct drm_i915_private {
struct intel_opregion opregion;
/* overlay */
- struct intel_overlay *overlay;
+ struct intel_legacy_overlay *overlay;
/* LVDS info */
int backlight_level; /* restore backlight to this value */
@@ -1309,8 +1309,8 @@ extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
/* overlay */
#ifdef CONFIG_DEBUG_FS
-extern struct intel_overlay_error_state
*intel_overlay_capture_error_state(struct drm_device *dev);
-extern void intel_overlay_print_error_state(struct seq_file *m, struct
intel_overlay_error_state *error);
+extern struct intel_legacy_overlay_error_state
*intel_legacy_overlay_capture_error_state(struct drm_device *dev);
+extern void intel_legacy_overlay_print_error_state(struct seq_file *m, struct
intel_legacy_overlay_error_state *error);
extern struct intel_display_error_state
*intel_display_capture_error_state(struct drm_device *dev);
extern void intel_display_print_error_state(struct seq_file *m,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9ee2729..36f2837 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -998,7 +998,7 @@ static void i915_capture_error_state(struct drm_device *dev)
do_gettimeofday(&error->time);
- error->overlay = intel_overlay_capture_error_state(dev);
+ error->overlay = intel_legacy_overlay_capture_error_state(dev);
error->display = intel_display_capture_error_state(dev);
spin_lock_irqsave(&dev_priv->error_lock, flags);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b1ae70b..e748338 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3180,7 +3180,7 @@ static void intel_crtc_dpms_overlay(struct intel_crtc
*intel_crtc, bool enable)
mutex_lock(&dev->struct_mutex);
dev_priv->mm.interruptible = false;
- (void) intel_overlay_switch_off(intel_crtc->overlay);
+ (void) intel_legacy_overlay_switch_off(intel_crtc->overlay);
dev_priv->mm.interruptible = true;
mutex_unlock(&dev->struct_mutex);
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 23c5622..467fb4a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -161,7 +161,7 @@ struct intel_crtc {
bool busy; /* is scanout buffer being updated frequently? */
struct timer_list idle_timer;
bool lowfreq_avail;
- struct intel_overlay *overlay;
+ struct intel_legacy_overlay *overlay;
struct intel_unpin_work *unpin_work;
int fdi_lanes;
@@ -370,7 +370,7 @@ extern void intel_finish_page_flip_plane(struct drm_device
*dev, int plane);
extern void intel_setup_overlay(struct drm_device *dev);
extern void intel_cleanup_overl
[PATCH 04/11] drm/i915: add SNB video sprite support
The video sprites support various video surface formats natively and can
handle scaling as well. So add support for them using the new DRM core
overlay support functions.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/Makefile |1 +
drivers/gpu/drm/i915/i915_reg.h | 52 +
drivers/gpu/drm/i915/intel_display.c | 25 +++-
drivers/gpu/drm/i915/intel_drv.h | 14 +++
drivers/gpu/drm/i915/intel_fb.c |6 +
drivers/gpu/drm/i915/intel_overlay2.c | 203 +
6 files changed, 294 insertions(+), 7 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0ae6a7c..6193471 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \
intel_dvo.o \
intel_ringbuffer.o \
intel_overlay.o \
+ intel_overlay2.o \
intel_opregion.o \
dvo_ch7xxx.o \
dvo_ch7017.o \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5a09416..7b128d4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2666,6 +2666,58 @@
#define _DSPBSURF 0x7119C
#define _DSPBTILEOFF 0x711A4
+/* Sprite A control */
+#define _DVSACNTR 0x72180
+#define DVS_ENABLE (1<<31)
+#define DVS_GAMMA_ENABLE (1<<30)
+#define DVS_PIXFORMAT_MASK (3<<25)
+#define DVS_FORMAT_YUV422(0<<25)
+#define DVS_FORMAT_RGBX101010(1<<25)
+#define DVS_FORMAT_RGBX888 (2<<25)
+#define DVS_FORMAT_RGBX161616(3<<25)
+#define DVS_SOURCE_KEY (1<<22)
+#define DVS_RGB_ORDER_RGBX (1<<20)
+#define DVS_YUV_BYTE_ORDER_MASK (3<<16)
+#define DVS_YUV_ORDER_YUYV (0<<16)
+#define DVS_YUV_ORDER_UYVY (1<<16)
+#define DVS_YUV_ORDER_YVYU (2<<16)
+#define DVS_YUV_ORDER_VYUY (3<<16)
+#define DVS_DEST_KEY (1<<2)
+#define DVS_TRICKLE_FEED_DISABLE (1<<14)
+#define DVS_TILED(1<<10)
+#define _DVSASTRIDE0x72188
+#define _DVSAPOS 0x7218c
+#define _DVSASIZE 0x72190
+#define _DVSAKEYVAL0x72194
+#define _DVSAKEYMSK0x72198
+#define _DVSASURF 0x7219c
+#define _DVSAKEYMAXVAL 0x721a0
+#define _DVSATILEOFF 0x721a4
+#define _DVSASURFLIVE 0x721ac
+#define _DVSASCALE 0x72204
+#define _DVSAGAMC 0x72300
+
+#define _DVSBCNTR 0x73180
+#define _DVSBSTRIDE0x73188
+#define _DVSBPOS 0x7318c
+#define _DVSBSIZE 0x73190
+#define _DVSBKEYVAL0x73194
+#define _DVSBKEYMSK0x73198
+#define _DVSBSURF 0x7319c
+#define _DVSBKEYMAXVAL 0x731a0
+#define _DVSBTILEOFF 0x731a4
+#define _DVSBSURFLIVE 0x731ac
+#define _DVSBSCALE 0x73204
+#define _DVSBGAMC 0x73300
+
+#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR)
+#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE)
+#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS)
+#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF)
+#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
+#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
+#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+
/* VBIOS regs */
#define VGACNTRL 0x71400
# define VGA_DISP_DISABLE (1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e748338..4f599ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -915,8 +915,8 @@ static void assert_panel_unlocked(struct drm_i915_private
*dev_priv,
pipe_name(pipe));
}
-static void assert_pipe(struct drm_i915_private *dev_priv,
- enum pipe pipe, bool state)
+void assert_pipe(struct drm_i915_private *dev_priv,
+enum pipe pipe, bool state)
{
int reg;
u32 val;
@@ -929,8 +929,6 @@ static void assert_pipe(struct drm_i915_private *dev_priv,
"pipe %c assertion failure (expected %s, current %s)\n",
pipe_name(pipe), state_string(state), state_string(cur_state));
}
-#define assert_pipe_enabled(d, p) assert_pipe(d, p, true)
-#define assert_pipe_disabled(d, p) assert_pipe(d, p, false)
static void assert_plane_enabled(struct drm_i915_private *dev_priv,
enum plane plane)
@@ -4439,7 +4437,8 @@ static void ironlake_update_wm(struct drm_device *dev)
ILK_LP0_CURSOR_LATENCY,
&plane_wm, &cursor_wm)) {
I915_WRITE(W
[PATCH 08/11] drm/i915: overlay watermark hack
---
drivers/gpu/drm/i915/intel_display.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4f599ce..cd7e04d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4521,7 +4521,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
&sandybridge_cursor_wm_info, latency,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEA_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe A -"
" plane %d, " "cursor: %d\n",
plane_wm, cursor_wm);
@@ -4533,7 +4534,8 @@ static void sandybridge_update_wm(struct drm_device *dev)
&sandybridge_cursor_wm_info, latency,
&plane_wm, &cursor_wm)) {
I915_WRITE(WM0_PIPEB_ILK,
- (plane_wm << WM0_PIPE_PLANE_SHIFT) | cursor_wm);
+ (plane_wm << WM0_PIPE_PLANE_SHIFT) |
+ (plane_wm << WM0_PIPE_SPRITE_SHIFT) | cursor_wm);
DRM_DEBUG_KMS("FIFO watermarks For pipe B -"
" plane %d, cursor: %d\n",
plane_wm, cursor_wm);
@@ -4587,11 +4589,6 @@ static void sandybridge_update_wm(struct drm_device *dev)
(plane_wm << WM1_LP_SR_SHIFT) |
cursor_wm);
-#if 0
- I915_WRITE(WM1S_LP_ILK,
- WM1S_LP_EN |
-#endif
-
/* WM2 */
if (!ironlake_compute_srwm(dev, 2, enabled,
SNB_READ_WM2_LATENCY() * 500,
--
1.7.4.1
[PATCH 06/11] drm/i915: plane teardown fixes
Make sure the object exists (it may not if the plane was previously disabled) and make sure we zero it out in the disable path to avoid trouble later. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index e583bd0..861e09e 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -149,12 +149,18 @@ intel_disable_plane(struct drm_plane *plane) mutex_lock(&dev->struct_mutex); + if (!intel_plane->obj) + goto out_unlock; + ret = i915_gem_object_finish_gpu(intel_plane->obj); if (ret) goto out_unlock; + i915_gem_object_unpin(intel_plane->obj); out_unlock: + intel_plane->obj = NULL; + I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE); I915_WRITE(DVSSURF(pipe), 0); POSTING_READ(DVSSURF(pipe)); -- 1.7.4.1
[PATCH 07/11] drm/i915: enable new overlay code on IVB too
Split things out a little and add the IVB reg definitions.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/i915_reg.h | 59
drivers/gpu/drm/i915/intel_overlay2.c | 168 ++--
2 files changed, 216 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7b128d4..71496b8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2718,6 +2718,65 @@
#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF)
+#define _SPRA_CTL 0x70280
+#define SPRITE_ENABLE(1<<31)
+#define SPRITE_GAMMA_ENABLE (1<<30)
+#define SPRITE_PIXFORMAT_MASK(7<<25)
+#define SPRITE_FORMAT_YUV422 (0<<25)
+#define SPRITE_FORMAT_RGBX101010 (1<<25)
+#define SPRITE_FORMAT_RGBX888(2<<25)
+#define SPRITE_FORMAT_RGBX161616 (3<<25)
+#define SPRITE_FORMAT_YUV444 (4<<25)
+#define SPRITE_FORMAT_XBGR101010 (5<<25)
+#define SPRITE_CSC_ENABLE(1<<24)
+#define SPRITE_SOURCE_KEY(1<<22)
+#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and
161616 */
+#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19)
+#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */
+#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16)
+#define SPRITE_YUV_ORDER_YUYV(0<<16)
+#define SPRITE_YUV_ORDER_UYVY(1<<16)
+#define SPRITE_YUV_ORDER_YVYU(2<<16)
+#define SPRITE_YUV_ORDER_VYUY(3<<16)
+#define SPRITE_TRICKLE_FEED_DISABLE (1<<14)
+#define SPRITE_INT_GAMMA_ENABLE (1<<13)
+#define SPRITE_TILED (1<<10)
+#define SPRITE_DEST_KEY (1<<2)
+#define _SPRA_STRIDE 0x70288
+#define _SPRA_POS 0x7028c
+#define _SPRA_SIZE 0x70290
+#define _SPRA_KEYVAL 0x70294
+#define _SPRA_KEYMSK 0x70298
+#define _SPRA_SURF 0x7029c
+#define _SPRA_KEYMAX 0x702a0
+#define _SPRA_TILEOFF 0x702a4
+#define _SPRA_SCALE0x70304
+#define _SPRA_GAMC 0x70400
+
+#define _SPRB_CTL 0x70280
+#define _SPRB_STRIDE 0x70288
+#define _SPRB_POS 0x7028c
+#define _SPRB_SIZE 0x70290
+#define _SPRB_KEYVAL 0x70294
+#define _SPRB_KEYMSK 0x70298
+#define _SPRB_SURF 0x7029c
+#define _SPRB_KEYMAX 0x702a0
+#define _SPRB_TILEOFF 0x702a4
+#define _SPRB_SCALE0x70304
+#define _SPRB_GAMC 0x70400
+
+#define SPRCTL(pipe) _PIPE(pipe, _SPRA_CTL, _SPRB_CTL)
+#define SPRSTRIDE(pipe) _PIPE(pipe, _SPRA_STRIDE, _SPRB_STRIDE)
+#define SPRPOS(pipe) _PIPE(pipe, _SPRA_POS, _SPRB_POS)
+#define SPRSIZE(pipe) _PIPE(pipe, _SPRA_SIZE, _SPRB_SIZE)
+#define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL)
+#define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK)
+#define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
+#define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
+#define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
+#define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
+#define SPRGAMC(pipe) _PIPE(pipe, _SPRA_GAMC, _SPRB_GAMC)
+
/* VBIOS regs */
#define VGACNTRL 0x71400
# define VGA_DISP_DISABLE (1 << 31)
diff --git a/drivers/gpu/drm/i915/intel_overlay2.c
b/drivers/gpu/drm/i915/intel_overlay2.c
index 861e09e..61b1a2f 100644
--- a/drivers/gpu/drm/i915/intel_overlay2.c
+++ b/drivers/gpu/drm/i915/intel_overlay2.c
@@ -36,7 +36,116 @@
#include "i915_drv.h"
static int
-intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
+ struct drm_framebuffer *fb, int crtc_x, int crtc_y,
+ unsigned int crtc_w, unsigned int crtc_h,
+ uint32_t src_x, uint32_t src_y,
+ uint32_t src_w, uint32_t src_h)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct intel_framebuffer *intel_fb;
+ struct drm_i915_gem_object *obj, *old_obj;
+ int pipe = intel_plane->pipe;
+ unsigned long start, offset;
+ u32 sprctl;
+ u32 reg = SPRCTL(pipe);
+ int ret = 0;
+ int x = src_x >> 16, y = src_y >> 16;
+
+ assert_pipe_enabled(dev_priv, pipe);
+
+ intel_fb = to_intel_framebuffer(fb);
+ obj = intel_fb->obj;
+
+ old_obj = intel_plane->obj;
+
+ mutex_lock(&dev-&g
[PATCH 09/11] drm/i915: fix overlay fb object handling
To avoid the object being destroyed before our disable hook is called,
take a private reference on it. This will guarantee that we can still
access the object at disable time.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/intel_overlay2.c | 27 ++-
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_overlay2.c
b/drivers/gpu/drm/i915/intel_overlay2.c
index 61b1a2f..8876857 100644
--- a/drivers/gpu/drm/i915/intel_overlay2.c
+++ b/drivers/gpu/drm/i915/intel_overlay2.c
@@ -35,6 +35,18 @@
#include "i915_drm.h"
#include "i915_drv.h"
+/*
+ * Note on refcounting:
+ * When the user creates an fb for the GEM object to be used for the plane,
+ * a ref is taken on the object. However, if the application exits before
+ * disabling the plane, the DRM close handling will free all the fbs and
+ * unless we take a ref on the object, it will be destroyed before the
+ * plane disable hook is called, causing obvious trouble with our efforts
+ * to look up and unpin the object. So we take a ref after we move the
+ * object to the display plane so it won't be destroyed until our disable
+ * hook is called and we drop our private reference.
+ */
+
static int
ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct drm_framebuffer *fb, int crtc_x, int crtc_y,
@@ -106,6 +118,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
if (ret)
goto out_unlock;
+ drm_gem_object_reference(&obj->base);
+
intel_plane->obj = obj;
sprctl |= SPRITE_TILED;
@@ -117,9 +131,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
start = obj->gtt_offset;
offset = y * fb->pitch + x * (fb->bits_per_pixel / 8);
- DRM_ERROR("enabling sprite, pos %d,%d, size %dx%d\n",
- crtc_x, crtc_y, fb->width, fb->height);
-
I915_WRITE(SPRSTRIDE(pipe), fb->pitch);
I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
@@ -215,6 +226,8 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
if (ret)
goto out_unlock;
+ drm_gem_object_reference(&obj->base);
+
intel_plane->obj = obj;
dvscntr |= DVS_TILED;
@@ -260,13 +273,15 @@ ivb_disable_plane(struct drm_plane *plane)
if (!intel_plane->obj)
goto out_unlock;
-#if 0
+
ret = i915_gem_object_finish_gpu(intel_plane->obj);
if (ret)
goto out_unlock;
i915_gem_object_unpin(intel_plane->obj);
-#endif
+
+ drm_gem_object_reference(&intel_plane->obj->base);
+
out_unlock:
intel_plane->obj = NULL;
@@ -299,6 +314,8 @@ snb_disable_plane(struct drm_plane *plane)
i915_gem_object_unpin(intel_plane->obj);
+ drm_gem_object_reference(&intel_plane->obj->base);
+
out_unlock:
intel_plane->obj = NULL;
--
1.7.4.1
[PATCH 11/11] drm/i915: add sprite scaling support
If the source and destination size are different, try to scale the sprite on the corresponding CRTC. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/i915_reg.h |5 + drivers/gpu/drm/i915/intel_overlay2.c | 14 -- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 71496b8..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2695,6 +2695,11 @@ #define _DVSATILEOFF 0x721a4 #define _DVSASURFLIVE 0x721ac #define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) #define _DVSAGAMC 0x72300 #define _DVSBCNTR 0x73180 diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 90c4f59..61594b6 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -169,7 +169,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct drm_i915_gem_object *obj, *old_obj; int pipe = intel_plane->pipe; unsigned long start, offset; - u32 dvscntr; + u32 dvscntr, dvsscale = 0; u32 reg = DVSCNTR(pipe); int ret = 0; int x = src_x >> 16, y = src_y >> 16; @@ -185,6 +185,13 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (crtc_x >= active_w || crtc_y >= active_h) return -EINVAL; + /* +* We can take a larger source and scale it down, but +* only so much... 16x is the max on SNB. +*/ + if (((src_w * src_h) / (crtc_w * crtc_h)) > 16) + return -EINVAL; + /* Clamp the width & height into the visible area */ if (crtc_x + crtc_w > active_w) crtc_w = active_w - crtc_x - 1; @@ -249,11 +256,14 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, start = obj->gtt_offset; offset = y * fb->pitch + x * (fb->bits_per_pixel / 8); + if (crtc_w != src_w || crtc_h != src_h) + dvsscale = DVS_SCALE_ENABLE | (src_h << 16) | src_w; + I915_WRITE(DVSSTRIDE(pipe), fb->pitch); I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x); I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x); I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); - I915_WRITE(DVSSCALE(pipe), 0); + I915_WRITE(DVSSCALE(pipe), dvsscale); I915_WRITE(reg, dvscntr); I915_WRITE(DVSSURF(pipe), start); POSTING_READ(DVSSURF(pipe)); -- 1.7.4.1
[PATCH 10/11] drm/i915: clamp sprite to viewable area
If we try to scan a sprite outside of the parent CRTC area, the display engine will underflow and potentially blank the framebuffer. So clamp the position + size to the viewable area. Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_overlay2.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_overlay2.c b/drivers/gpu/drm/i915/intel_overlay2.c index 8876857..90c4f59 100644 --- a/drivers/gpu/drm/i915/intel_overlay2.c +++ b/drivers/gpu/drm/i915/intel_overlay2.c @@ -173,6 +173,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, u32 reg = DVSCNTR(pipe); int ret = 0; int x = src_x >> 16, y = src_y >> 16; + int active_w = crtc->mode.hdisplay, active_h = crtc->mode.vdisplay; assert_pipe_enabled(dev_priv, pipe); @@ -181,6 +182,15 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, old_obj = intel_plane->obj; + if (crtc_x >= active_w || crtc_y >= active_h) + return -EINVAL; + + /* Clamp the width & height into the visible area */ + if (crtc_x + crtc_w > active_w) + crtc_w = active_w - crtc_x - 1; + if (crtc_y + crtc_h > active_h) + crtc_h = active_h - crtc_y - 1; + mutex_lock(&dev->struct_mutex); dvscntr = I915_READ(reg); @@ -242,7 +252,7 @@ snb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, I915_WRITE(DVSSTRIDE(pipe), fb->pitch); I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x); I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x); - I915_WRITE(DVSSIZE(pipe), (fb->height << 16) | fb->width); + I915_WRITE(DVSSIZE(pipe), (crtc_h << 16) | crtc_w); I915_WRITE(DVSSCALE(pipe), 0); I915_WRITE(reg, dvscntr); I915_WRITE(DVSSURF(pipe), start); -- 1.7.4.1
DRM planes and new fb creation ioctl
On Tue, 25 Oct 2011 19:47:13 +0900 Joonyoung Shim wrote: > 10/25/2011 06:46 PM, Jesse Barnes ? ?: > > I've given up waiting for someone to implement support for these > > ioctls on another platform before they're merged, but I have > > received a lot of feedback on the interfaces, and it sounds like > > they're ok. I've also fixed all the remaining issues I'm aware of > > on SNB platforms and things are working well, so I'm just going to > > push them out. (Note IVB support is still missing a few bits for > > scaling and such; I'll fix those up when I get back home and can > > test on IVB again.) > > > > One change you may notice from the last set is that I've removed the > > 'zpos' parameter. Plane blending and z ordering is very chipset > > specific (it even varies between Intel chipsets), so exposing it > > through a device specific ioctl is probably a better plan. > > But i think zpos is essential parameter of plane. If plane doesn't > support it, drm driver cannot know user wants to use which overlay, > so i wonder what it meant DRM_IOCTL_MODE_SETPLANE zpos is absent . Setplane is just for attaching a new fb. The order, keying, or whatever else your plane blender can support can be set with a device specific ioctl before or after the setplane call (probably before to avoid any flashing or bad frames). > If use device specific ioctl, should implement device specific ioctl > for DRM_IOCTL_MODE_SETPLANE? You could if you needed, but I don't think it's strictly necessary. > >By default, planes > > should just overlay the primary plane; a device specific ioctl (none > > available yet, but I have some planned for i915) can provide more > > flexibility. > > Could you explain what is the primary plane? Is it same as the overlay > handled by crtc? It confuses a bit when one overlay is handled by crtc > and plane at the same time. Yeah, it is a little confusing. When I refer to the primary, I'm referring to the plane bound to the CRTC. I'm fine if someone wants to break that out, I think it would make sense. I just didn't want to write the compat code that would be required for that scheme. :) But these patches definitely don't preclude it, and I don't think these interfaces will need changes if we ever move to a pipe/plane split at the userland level. Thanks, Jesse
[PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 19:53:02 +0900
Joonyoung Shim wrote:
> > +/**
> > + * drm_plane - central DRM plane control structure
> > + * @dev: DRM device this plane belongs to
> > + * @kdev: kernel device
> > + * @attr: kdev attributes
> > + * @head: for list management
> > + * @base: base mode object
> > + * @crtc_x: x position of plane (relative to pipe base)
> > + * @crtc_y: y position of plane
> > + * @x: x offset into fb
> > + * @y: y offset into fb
> Above 4 members don't be used.
Oops yeah, I'll fix up the comments.
> > +
> > +struct drm_mode_get_plane {
> > + __u64 format_type_ptr;
> > + __u32 plane_id;
> > +
> > + __u32 crtc_id;
> > + __u32 fb_id;
> > +
> > + __u32 possible_crtcs;
> > + __u32 gamma_size;
> > +
> > + __u32 count_format_types;
> > +};
>
> I wonder why doesn't give to user crtc_x, crtc_y, crtc_w, crtc_h
> information?
It could, but the caller should already know was my thinking. Would
you like those bits returned?
Jesse
[Intel-gfx] DRM planes and new fb creation ioctl
On Tue, 25 Oct 2011 11:46:55 +0200 Jesse Barnes wrote: > I've given up waiting for someone to implement support for these > ioctls on another platform before they're merged, but I have received > a lot of feedback on the interfaces, and it sounds like they're ok. > I've also fixed all the remaining issues I'm aware of on SNB > platforms and things are working well, so I'm just going to push them > out. (Note IVB support is still missing a few bits for scaling and > such; I'll fix those up when I get back home and can test on IVB > again.) Btw, ignore 3-11, I forgot to collapse them when I collapsed the core fixes. Updated intel specific patch will be coming shortly (with a fix for the unpin vs disable ordering danvet pointed out). Jesse
[PATCH] drm/i915: add SNB video sprite support
The video sprites support various video surface formats natively and can handle scaling as well. So add support for them using the new DRM core overlay support functions. v2: collapse patches and fix plane disable vs unpin ordering bug Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_reg.h | 116 ++ drivers/gpu/drm/i915/intel_display.c | 26 ++- drivers/gpu/drm/i915/intel_drv.h | 14 ++ drivers/gpu/drm/i915/intel_fb.c |6 + drivers/gpu/drm/i915/intel_overlay2.c | 393 + 6 files changed, 547 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0ae6a7c..6193471 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \ intel_dvo.o \ intel_ringbuffer.o \ intel_overlay.o \ + intel_overlay2.o \ intel_opregion.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a09416..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2666,6 +2666,122 @@ #define _DSPBSURF 0x7119C #define _DSPBTILEOFF 0x711A4 +/* Sprite A control */ +#define _DVSACNTR 0x72180 +#define DVS_ENABLE (1<<31) +#define DVS_GAMMA_ENABLE (1<<30) +#define DVS_PIXFORMAT_MASK (3<<25) +#define DVS_FORMAT_YUV422(0<<25) +#define DVS_FORMAT_RGBX101010(1<<25) +#define DVS_FORMAT_RGBX888 (2<<25) +#define DVS_FORMAT_RGBX161616(3<<25) +#define DVS_SOURCE_KEY (1<<22) +#define DVS_RGB_ORDER_RGBX (1<<20) +#define DVS_YUV_BYTE_ORDER_MASK (3<<16) +#define DVS_YUV_ORDER_YUYV (0<<16) +#define DVS_YUV_ORDER_UYVY (1<<16) +#define DVS_YUV_ORDER_YVYU (2<<16) +#define DVS_YUV_ORDER_VYUY (3<<16) +#define DVS_DEST_KEY (1<<2) +#define DVS_TRICKLE_FEED_DISABLE (1<<14) +#define DVS_TILED(1<<10) +#define _DVSASTRIDE0x72188 +#define _DVSAPOS 0x7218c +#define _DVSASIZE 0x72190 +#define _DVSAKEYVAL0x72194 +#define _DVSAKEYMSK0x72198 +#define _DVSASURF 0x7219c +#define _DVSAKEYMAXVAL 0x721a0 +#define _DVSATILEOFF 0x721a4 +#define _DVSASURFLIVE 0x721ac +#define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) +#define _DVSAGAMC 0x72300 + +#define _DVSBCNTR 0x73180 +#define _DVSBSTRIDE0x73188 +#define _DVSBPOS 0x7318c +#define _DVSBSIZE 0x73190 +#define _DVSBKEYVAL0x73194 +#define _DVSBKEYMSK0x73198 +#define _DVSBSURF 0x7319c +#define _DVSBKEYMAXVAL 0x731a0 +#define _DVSBTILEOFF 0x731a4 +#define _DVSBSURFLIVE 0x731ac +#define _DVSBSCALE 0x73204 +#define _DVSBGAMC 0x73300 + +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR) +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE) +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS) +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF) +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE) +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) + +#define _SPRA_CTL 0x70280 +#define SPRITE_ENABLE(1<<31) +#define SPRITE_GAMMA_ENABLE (1<<30) +#define SPRITE_PIXFORMAT_MASK(7<<25) +#define SPRITE_FORMAT_YUV422 (0<<25) +#define SPRITE_FORMAT_RGBX101010 (1<<25) +#define SPRITE_FORMAT_RGBX888(2<<25) +#define SPRITE_FORMAT_RGBX161616 (3<<25) +#define SPRITE_FORMAT_YUV444 (4<<25) +#define SPRITE_FORMAT_XBGR101010 (5<<25) +#define SPRITE_CSC_ENABLE(1<<24) +#define SPRITE_SOURCE_KEY(1<<22) +#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and 161616 */ +#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19) +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */ +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16) +#define SPRITE_YUV_ORDER_YUYV(0<<16) +#define SPRITE_YUV_ORDER_UYVY(1<<16) +#define SPRITE_YUV_ORDER_YVYU
[PATCH] drm/i915: add SNB video sprite support
[PATCH] drm/i915: add SNB video sprite support
The video sprites support various video surface formats natively and can handle scaling as well. So add support for them using the new DRM core overlay support functions. v2: collapse patches v3: no really, fix disable ordering Signed-off-by: Jesse Barnes --- drivers/gpu/drm/i915/Makefile |1 + drivers/gpu/drm/i915/i915_reg.h | 116 ++ drivers/gpu/drm/i915/intel_display.c | 26 ++- drivers/gpu/drm/i915/intel_drv.h | 14 ++ drivers/gpu/drm/i915/intel_fb.c |6 + drivers/gpu/drm/i915/intel_overlay2.c | 395 + 6 files changed, 549 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_overlay2.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 0ae6a7c..6193471 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o i915_mem.o \ intel_dvo.o \ intel_ringbuffer.o \ intel_overlay.o \ + intel_overlay2.o \ intel_opregion.o \ dvo_ch7xxx.o \ dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a09416..8cbda0b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -2666,6 +2666,122 @@ #define _DSPBSURF 0x7119C #define _DSPBTILEOFF 0x711A4 +/* Sprite A control */ +#define _DVSACNTR 0x72180 +#define DVS_ENABLE (1<<31) +#define DVS_GAMMA_ENABLE (1<<30) +#define DVS_PIXFORMAT_MASK (3<<25) +#define DVS_FORMAT_YUV422(0<<25) +#define DVS_FORMAT_RGBX101010(1<<25) +#define DVS_FORMAT_RGBX888 (2<<25) +#define DVS_FORMAT_RGBX161616(3<<25) +#define DVS_SOURCE_KEY (1<<22) +#define DVS_RGB_ORDER_RGBX (1<<20) +#define DVS_YUV_BYTE_ORDER_MASK (3<<16) +#define DVS_YUV_ORDER_YUYV (0<<16) +#define DVS_YUV_ORDER_UYVY (1<<16) +#define DVS_YUV_ORDER_YVYU (2<<16) +#define DVS_YUV_ORDER_VYUY (3<<16) +#define DVS_DEST_KEY (1<<2) +#define DVS_TRICKLE_FEED_DISABLE (1<<14) +#define DVS_TILED(1<<10) +#define _DVSASTRIDE0x72188 +#define _DVSAPOS 0x7218c +#define _DVSASIZE 0x72190 +#define _DVSAKEYVAL0x72194 +#define _DVSAKEYMSK0x72198 +#define _DVSASURF 0x7219c +#define _DVSAKEYMAXVAL 0x721a0 +#define _DVSATILEOFF 0x721a4 +#define _DVSASURFLIVE 0x721ac +#define _DVSASCALE 0x72204 +#define DVS_SCALE_ENABLE (1<<31) +#define DVS_FILTER_MASK (3<<29) +#define DVS_FILTER_MEDIUM(0<<29) +#define DVS_FILTER_ENHANCING (1<<29) +#define DVS_FILTER_SOFTENING (2<<29) +#define _DVSAGAMC 0x72300 + +#define _DVSBCNTR 0x73180 +#define _DVSBSTRIDE0x73188 +#define _DVSBPOS 0x7318c +#define _DVSBSIZE 0x73190 +#define _DVSBKEYVAL0x73194 +#define _DVSBKEYMSK0x73198 +#define _DVSBSURF 0x7319c +#define _DVSBKEYMAXVAL 0x731a0 +#define _DVSBTILEOFF 0x731a4 +#define _DVSBSURFLIVE 0x731ac +#define _DVSBSCALE 0x73204 +#define _DVSBGAMC 0x73300 + +#define DVSCNTR(pipe) _PIPE(pipe, _DVSACNTR, _DVSBCNTR) +#define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE) +#define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS) +#define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF) +#define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE) +#define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE) +#define DVSTILEOFF(pipe) _PIPE(pipe, _DVSATILEOFF, _DVSBTILEOFF) + +#define _SPRA_CTL 0x70280 +#define SPRITE_ENABLE(1<<31) +#define SPRITE_GAMMA_ENABLE (1<<30) +#define SPRITE_PIXFORMAT_MASK(7<<25) +#define SPRITE_FORMAT_YUV422 (0<<25) +#define SPRITE_FORMAT_RGBX101010 (1<<25) +#define SPRITE_FORMAT_RGBX888(2<<25) +#define SPRITE_FORMAT_RGBX161616 (3<<25) +#define SPRITE_FORMAT_YUV444 (4<<25) +#define SPRITE_FORMAT_XBGR101010 (5<<25) +#define SPRITE_CSC_ENABLE(1<<24) +#define SPRITE_SOURCE_KEY(1<<22) +#define SPRITE_RGB_ORDER_RGBX(1<<20) /* only for 888 and 161616 */ +#define SPRITE_YUV_TO_RGB_CSC_DISABLE(1<<19) +#define SPRITE_YUV_CSC_FORMAT_BT709 (1<<18) /* 0 is BT601 */ +#define SPRITE_YUV_BYTE_ORDER_MASK (3<<16) +#define SPRITE_YUV_ORDER_YUYV(0<<16) +#define SPRITE_YUV_ORDER_UYVY(1<<16) +#define SPRITE_YUV_ORDER_YVYU
[PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 13:58:55 +0200 Daniel Vetter wrote: > On Tue, Oct 25, 2011 at 11:46:56AM +0200, Jesse Barnes wrote: > > Planes are a bit like half-CRTCs. They have a location and fb, but > > don't drive outputs directly. Add support for handling them to the > > core KMS code. > > > > Signed-off-by: Jesse Barnes > > As discussed with Jesse on irc, drm fb handling is fragile. Current > rules: > - fbs are not reference counted, hence when destroying we need to > disable all crtcs (and now also planes) that use them. > drm_framebuffer_cleanup does that atm > - drivers that hold onto fbs after the kms core drops the > corresponding pointer needs to hold a ref onto the underlying backing > storage (like e.g. for pageflip on the to-be-flipped-out fb as long > as it might still be scanned out). > > We need proper refcounting for these ... But for now this patch is > missing the plane cleanup in drm_framebuffer_cleanup. Ah yeah that's a better place for the disable plane call I currently have in the intel specific code... I'll fix up and test. > Otherwise I think going with just the src and dst rect for set_plane > is about the only sensible thing given the crazy hw out there. But I > lack the knowledge about that kind of hw (and video stuff in > general), so I'll refrain from slapping my r-b on these two. Yeah, I think the drivers just need to be able to calculate the scaling level from the params and program them into whatever regs they happen to have (on Intel fortunately the scaling is figured out by hw when we program in the source & dest values, subject to some restrictions). Thanks, Jesse
[PATCH 01/11] drm: add plane support
On Tue, 25 Oct 2011 14:26:07 +0100 Alan Cox wrote: > > As discussed with Jesse on irc, drm fb handling is fragile. Current > > rules: > > - fbs are not reference counted, hence when destroying we need to > > disable all crtcs (and now also planes) that use them. > > drm_framebuffer_cleanup does that atm > > - drivers that hold onto fbs after the kms core drops the > > corresponding pointer needs to hold a ref onto the underlying > > backing storage (like e.g. for pageflip on the to-be-flipped-out fb > > as long as it might still be scanned out). > > > > We need proper refcounting for these ... But for now this patch is > > missing the plane cleanup in drm_framebuffer_cleanup. > > I'd rather we fixed the framebuffer kref stuff as part of doing this > rather than have a poorer API because of something we have to fix > anyway. > > It shouldn't be *that* hard to fix, at least for this kind of use > case. Resize locking, fb moving etc are ugly issues, refcount > shouldn't be, and the tty layer also refcounts so we can only have > the fb objects themselves to worry about as we can defer fb > destruction to tty close or drm last unref even for those with > consoles on them. Oh it doesn't affect the userspace API so I don't think it's urgent. But I agree, it would be nice to clean up fb management a bit... Any volunteers? :) Thanks, Jesse
[PATCH 01/11] drm: add plane support
Here's a diff I can roll in if it looks ok. It adds the ability to
specify multiple handles for a single fb to better accommodate planar
configs. I think Rob has convinced me that this is a good idea...
comments appreciated.
Thanks,
Jesse
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a30b9d4..0cc2077 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1923,7 +1923,8 @@ int drm_mode_addfb(struct drm_device *dev,
r.bpp = or->bpp;
r.depth = or->depth;
r.pixel_format = 0;
- r.handle = or->handle;
+ r.handle_count = 1;
+ r.handles = (u64)&or->handle;
if (!drm_core_check_feature(dev, DRIVER_MODESET))
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cd7e04d..2c7f200 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7619,8 +7619,9 @@ intel_user_framebuffer_create(struct drm_device *dev,
struct drm_mode_fb_cmd2 *mode_cmd)
{
struct drm_i915_gem_object *obj;
+ u32 *handles = (u32 *)mode_cmd->handles;
- obj = to_intel_bo(drm_gem_object_lookup(dev, filp, mode_cmd->handle));
+ obj = to_intel_bo(drm_gem_object_lookup(dev, filp, handles[0]));
if (&obj->base == NULL)
return ERR_PTR(-ENOENT);
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
b/drivers/gpu/drm/nouveau/nouveau_display.c
index 7a428a9..cb9b868 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -128,9 +128,10 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
{
struct nouveau_framebuffer *nouveau_fb;
struct drm_gem_object *gem;
+ u32 *handles = (u32 *)mode_cmd->handles;
int ret;
- gem = drm_gem_object_lookup(dev, file_priv, mode_cmd->handle);
+ gem = drm_gem_object_lookup(dev, file_priv, handles);
if (!gem)
return ERR_PTR(-ENOENT);
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index ae803f8..63a6d91 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -1128,11 +1128,12 @@ radeon_user_framebuffer_create(struct drm_device *dev,
{
struct drm_gem_object *obj;
struct radeon_framebuffer *radeon_fb;
+ u32 *handles = (u32 *)mode_cmd->handles;
- obj = drm_gem_object_lookup(dev, file_priv, mode_cmd->handle);
+ obj = drm_gem_object_lookup(dev, file_priv, handles[0]);
if (obj == NULL) {
dev_err(&dev->pdev->dev, "No GEM object associated to handle
0x%08X, "
- "can't create framebuffer\n", mode_cmd->handle);
+ "can't create framebuffer\n", handles[0]);
return ERR_PTR(-ENOENT);
}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 2a1b802..0ad7456 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -866,7 +866,7 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct
drm_device *dev,
*/
ret = vmw_user_surface_lookup_handle(dev_priv, tfile,
-mode_cmd->handle, &surface);
+mode_cmd->handles[0], &surface);
if (ret)
goto try_dmabuf;
diff --git a/drivers/staging/gma500/framebuffer.c
b/drivers/staging/gma500/framebuffer.c
index 85f47d5..ee91ffe 100644
--- a/drivers/staging/gma500/framebuffer.c
+++ b/drivers/staging/gma500/framebuffer.c
@@ -496,7 +496,7 @@ static struct drm_framebuffer *psb_user_framebuffer_create
* Find the GEM object and thus the gtt range object that is
* to back this space
*/
- obj = drm_gem_object_lookup(dev, filp, cmd->handle);
+ obj = drm_gem_object_lookup(dev, filp, cmd->handles[0]);
if (obj == NULL)
return ERR_PTR(-ENOENT);
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 34a0d22..dafe8df 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -272,8 +272,9 @@ struct drm_mode_fb_cmd2 {
__u32 bpp;
__u32 depth;
__u32 pixel_format; /* fourcc code from videodev2.h */
- /* driver specific handle */
- __u32 handle;
+ __u32 handle_count;
+ /* driver specific buffer object handle array */
+ __u64 handles;
};
#define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01
[PATCH 01/11] drm: add plane support
On Wed, 26 Oct 2011 14:40:07 +0900
Joonyoung Shim wrote:
> 10/25/2011 06:46 PM, Jesse Barnes ? ?:
>
> [snip]
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index 8020798..d7f03aa 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -44,6 +44,7 @@ struct drm_framebuffer;
> > #define DRM_MODE_OBJECT_PROPERTY 0xb0b0b0b0
> > #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
> > #define DRM_MODE_OBJECT_BLOB 0x
> > +#define DRM_MODE_OBJECT_PLANE 0x
> >
> > struct drm_mode_object {
> > uint32_t id;
> > @@ -278,6 +279,7 @@ struct drm_crtc;
> > struct drm_connector;
> > struct drm_encoder;
> > struct drm_pending_vblank_event;
> > +struct drm_plane;
> >
> > /**
> >* drm_crtc_funcs - control CRTCs for a given device
> > @@ -536,6 +538,58 @@ struct drm_connector {
> > };
> >
> > /**
> > + * drm_plane_funcs - driver plane control functions
> > + * @update_plane: update the plane configuration
> > + */
> > +struct drm_plane_funcs {
> > + int (*update_plane)(struct drm_plane *plane,
> > + struct drm_crtc *crtc, struct drm_framebuffer *fb,
> > + int crtc_x, int crtc_y,
> > + unsigned int crtc_w, unsigned int crtc_h,
> > + uint32_t src_x, uint32_t src_y,
> > + uint32_t src_w, uint32_t src_h);
> > + int (*disable_plane)(struct drm_plane *plane);
> > +};
>
> How about add destroy() function and call it in
> drm_mode_config_cleanup()?
Oh good idea; destroy will be needed for hot plug.
--
Jesse Barnes, Intel Open Source Technology Center
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20111027/347bd6d2/attachment.pgp>
Proposal for a low-level Linux display framework
On Sat, 17 Sep 2011 21:25:29 +0100 Alan Cox wrote: > > Just tell the X driver to not use acceleration, and it you won't get > > any acceleration used, then you get complete stability. If a driver > > writer wants to turn off all accel in the kernel driver, it can, its > > In fact one thing we actually need really is a "dumb" KMS X server to > replace the fbdev X server that unaccel stuff depends upon and which > can't do proper mode handling, multi-head or resizing as a result. A dumb > fb generic request for a back to front copy might also be useful for > shadowfb, or at least indicators so you know what the cache behaviour is > so the X server can pick the right policy. > > > We've fixed this in KMS, we don't pass direct mappings to userspace > > that we can't tear down and refault. We only provide objects via > > handles. The only place its a problem is where we expose fbdev legacy > > emulation, since we have to fix the pages. > > Which is doable. Horrible but doable. The usb framebuffer code has to > play games like this with the virtual framebuffer in order to track > changes by faulting. > > There are still some architectural screwups however. DRM continues the > fbdev worldview that outputs, memory and accelerators are tied together > in lumps we call video cards. That isn't really true for all cases and > with capture/overlay it gets even less true. Sorry for re-opening this ancient thread; I'm catching up from the past 2 months of travel & misc. I definitely agree about the PC card centric architecture of DRM KMS (and before it, X). But we have a path out of it now, and lots of interest from vendors and developers, so I don't think it's an insurmountable problem by any means. I definitely understand Florian's worries about DRM vs fb. If nothing else, there's certainly a perception that fb is simpler and easier to get right. But really, as others have pointed out, it's solving a different set of problems than the DRM layer. The latter is actually trying to expose the features of contemporary hardware in a way that's as portable as possible. That portability comes at a cost though: the APIs we add need to get lots of review, and there's no doubt we'll need to add more as newer, weirder hardware comes along. Really, I see no reason why fb and DRM can't continue to live side by side. If a vendor really only needs the features provided by the fb layer, they're free to stick with a simple fb driver. However, I expect most vendors making phones, tablets, notebooks, etc will need and want an architecture that looks a lot like the DRM layer, with authentication for rendering clients, an command submission ioctl for acceleration, and memory management, so I expect most of the driver growth to be in DRM in the near future. And I totally agree with Dave about having a kmscon. I really wish someone would implement it so I could have my VTs spinning on a cube. -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20111031/8a7fa5bb/attachment-0001.pgp>
[Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel
On Mon, 19 Sep 2011 15:22:00 -0700 Keith Packard wrote: > The eDP panel may not be able to respond to aux channel communications > unless it has power supplied. During mode setting, power may be > cut-off during panel power sequencing. Make sure that any aux channel > communications will work by forcing vdd power active as needed. > > This also delays after turning power on and off to ensure that the > panel is keeping up. > > Signed-off-by: Keith Packard This one mixes up lots of cleanups plus the EDID read with the power changes. I'm worried about the VDD smashing as well; we have lots of bugs in the PPS hardware around VDD vs full PPS. We need to make sure appropriate delays are in place when transitioning from one to another. In what paths are we trying to do accesses without power applied? Looks like mainly edid? I see the next patch handles the timing stuff, I assume it's ok. Jesse
[Intel-gfx] [PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously
On Mon, 19 Sep 2011 15:22:03 -0700 Keith Packard wrote: > There's no good reason to turn off the eDP force VDD bit synchronously > while probing devices; that just sticks a huge delay into all mode > setting paths. Instead, queue a delayed work proc to disable the VDD > force bit and then remember when that fires to ensure that the > appropriate delay is respected before trying to turn it back on. > > Signed-off-by: Keith Packard > --- I'm worried this makes our PPS even more complex and hard to follow. I'd rather see VDD AUX applied only when we need it (dpms, mode set and detect; for hotplug we can assume the panel is alive) and that we carefully disable it after waiting for a time after a full PPS on sequence when doing a mode set or dpms on command. Having all the power stuff at the highest levels would be clearest I think. Jesse
[Intel-gfx] [PATCH 6/9] drm/i915: Make sure eDP power is on before using aux channel
On Tue, 20 Sep 2011 21:45:54 -0700 Keith Packard wrote: > On Wed, 21 Sep 2011 09:20:01 +0530, Jesse Barnes > wrote: > > > This one mixes up lots of cleanups plus the EDID read with the power > > changes. > > I think the cleanups are; > > 1) edp checks inside vdd_on and vdd_off to make the other code a bit > easier to read. > > 2) Hold VDD on until the end of dp_commit. I think this isn't > necessary and could be removed -- once the panel is on, we don't need > to hold vdd up. > > 3) Hold VDD up through the whole dpms sequence. Also probably > unnecessary. > > 4) Move intel_dp_i2c_init past the computation of the power > sequencing delays. Necessary as i2c_init makes an i2c transaction, > thus powering up the panel. > > I can remove the middle two and split the others out if you like. Yeah that sounds good. (2) and (3) are ok cleanups, but it would be best if they were a separate patch just in case the subtle timing change breaks the panel power sequencing state machine. > > I'm worried about the VDD smashing as well; we have lots of > > bugs in the PPS hardware around VDD vs full PPS. > > Are you worried that we should never have VDD up while running a panel > power sequence? As far as I can tell from the eDP specs, bringing VDD > up is part of the normal PPS, and the delay from VDD up to other panel > sequencing is shorter (T1+T2) than the delay to start using the aux > channel that I used in the later patch (T1+T2+T3). No I think: 1) VDD AUX override on 2) PPS on 3) (delay) 4) VDD AUX override off is safe, I'm just worried about the timing of step (3). > One thing I learned for certain -- we don't want a synchronous delay > between turning the panel off and back on. The required interval > between these two operations is in units of 100ms; my machine spends > 600ms doing this; if we end up doing this in the middle of a mode > set, it's gonna be painful. Agree, speeding that up would be nice. > Can we replace any of the current VDD hacks with a full PPS? I can > easily imagine moving the call to ironlake_edp_panel_on to > intel_dp_prepare, except that if if the mode_set fails, dp_commit will > not be called (that looks like a potential source of failure at the > DRM level to me). Ah yeah this brings back the memories (or is it PTS?). To fix both PCH eDP and CPU eDP in the past, I did have a version that only used full PPS and not VDD AUX override. So it is possible, but VDD AUX is a little cleaner since it allows us to keep the registers locked potentially (theoretically we only actually want to unlock to handle CPU eDP PLL enable/disable bugs and flicker-free panel fitter downscaling). But since we unlock unconditionally now, using full PPS would be ok. > Getting the panel turned on complete as early as possible seems like a > good idea, instead of fussing around with the VDD force bits. Though we will have to shut it down still; PPS on to get AUX data and EDID, then off while we program the mode and train DP, then PPS on again. So I'm not sure it would save much. > Alternatively, we can eliminate use of the VDD force hack and do a > full PPS and simply use the delayed work proc to turn that off when > the screen goes idle. Yeah, I'm liking your delayed work much better now... Bring up the panel early and then just modify the shutdown timeout at various points to make sure it stays up from module_init all the way through the final mode set (so an initial timeout of 2s or so would probably be sufficient). Another potential optimization is to start trying AUX & i2c transactions right after we apply VDD AUX override. The panel will come up much faster than the T* values imply most of the time (varies by panel). And polling the bits can get us into the actual panel poking code much faster. But I think the bottom line is to fix the EDID read (make sure the panel is on) and the i2c stuff. Everything else is just tasty gravy. :) Also I think the change to prefer EDID over VBT is correct; afaik eDP panels are required to have an EDID, so trusting that data over some potentially untested VBT data is the right way to go. Thanks, Jesse
[Intel-gfx] [PATCH 9/9] drm/i915: Disable eDP VDD in a delayed work proc instead of synchronously
On Tue, 20 Sep 2011 21:51:33 -0700 Keith Packard wrote: > Yes, making it cleaner would help a ton. There are some basic problems > with the DRM API that make this hard though -- intel_dp_prepare may > not ever be followed by a call to intel_dp_commit. That's why I had > the VDD AUX stuff get turned off by a delayed work proc instead. > > Also, leaving VDD AUX high after EDID is fetched means that we can > start the mode setting immediately, rather than having to wait for the > power-off/power-on delay (which is really long). Yeah agreed. > What we could do is force VDD AUX off after the panel gets turned on; > that would ensure that turning the panel off would actually turn the > power off, rather than having VDD stay high for some time after that. Yep, that sounds great. Thanks, Jesse
i915_driver_irq_handler: irq 42: nobody cared
On Fri, 30 Mar 2012 11:45:43 +0100 Chris Wilson wrote: > On Fri, 30 Mar 2012 11:59:28 +0200, Jiri Slaby wrote: > > I don't know what to dump more, because iir is obviously zero too. What > > other sources of interrupts are on the (G33) chip? > > IIR is the master interrupt, with chained secondary interrupt statuses. > If IIR is 0, the interrupt wasn't raised by the GPU. I've actually seen cases where one of the PIPE*STAT regs is stuck, and even if IIR is 0 we still get interrupts... Jiri can you verify the PIPE*STAT regs have bits set, maybe one or more we don't check for? -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120409/f50f7a99/attachment.pgp>
i915_driver_irq_handler: irq 42: nobody cared [generic IRQ handling broken?]
On Sat, 7 Apr 2012 00:40:28 +0200 (CEST) Thomas Gleixner wrote: > You know what? suspend calls free_irq() via i915_drm_freeze() -> > drm_irq_uninstall() and the resume code calls request_irq() again. > free_irq() removes the action and request_irq installs it fresh. Yeah this is a known issue with the DRM code, I thought Dave had a fix queued a long time ago though... Dave? -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120409/f2bc660f/attachment.pgp>
[Intel-gfx] [PATCH 1/4] drm/i915: handle input/output sdvo timings separately in mode_set
de(&input_dtd, adjusted_mode);
> - (void) intel_sdvo_set_output_timing(intel_sdvo, &input_dtd);
> - }
> + /* lvds has a special fixed output timing. */
> + if (intel_sdvo->is_lvds)
> + intel_sdvo_get_dtd_from_mode(&output_dtd,
> + intel_sdvo->sdvo_lvds_fixed_mode);
> + else
> + intel_sdvo_get_dtd_from_mode(&output_dtd, mode);
> + (void) intel_sdvo_set_output_timing(intel_sdvo, &output_dtd);
>
> /* Set the input timing to the screen. Assume always input 0. */
> if (!intel_sdvo_set_target_input(intel_sdvo))
> @@ -1054,6 +1052,10 @@ static void intel_sdvo_mode_set(struct drm_encoder
> *encoder,
> !intel_sdvo_set_tv_format(intel_sdvo))
> return;
>
> + /* We have tried to get input timing in mode_fixup, and filled into
> + * adjusted_mode.
> + */
> + intel_sdvo_get_dtd_from_mode(&input_dtd, adjusted_mode);
> (void) intel_sdvo_set_input_timing(intel_sdvo, &input_dtd);
>
> switch (pixel_multiplier) {
But seems mostly separate from this hunk, which I don't really
understand, not being an SDVO expert.
What happened to the tv check? Is input_dtd already set to the right
value here after the change?
--
Jesse Barnes, Intel Open Source Technology Center
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120410/b641970f/attachment.pgp>
i915_driver_irq_handler: irq 42: nobody cared
On Tue, 10 Apr 2012 10:47:49 +0200 Jiri Slaby wrote: > On 04/09/2012 07:11 PM, Jesse Barnes wrote: > > On Fri, 30 Mar 2012 11:45:43 +0100 Chris Wilson > > wrote: > > > >> On Fri, 30 Mar 2012 11:59:28 +0200, Jiri Slaby > >> wrote: > >>> I don't know what to dump more, because iir is obviously zero > >>> too. What other sources of interrupts are on the (G33) chip? > >> > >> IIR is the master interrupt, with chained secondary interrupt > >> statuses. If IIR is 0, the interrupt wasn't raised by the GPU. > > > > I've actually seen cases where one of the PIPE*STAT regs is stuck, > > and even if IIR is 0 we still get interrupts... Jiri can you verify > > the PIPE*STAT regs have bits set, maybe one or more we don't check > > for? > > Note that I already attached their contents... This is what is in them > (pipes 0 and 1): > [ 3572.968581] i915_driver_irq_handler: 0= 1= > [ 3572.977472] i915_driver_irq_handler: 0= 1= > [ 3576.224839] i915_driver_irq_handler: 0= 1= > [ 3576.243558] i915_driver_irq_handler: 0= 1= > [ 3576.384912] i915_driver_irq_handler: 0= 1= > [ 3576.403462] i915_driver_irq_handler: 0= 1= > [ 3577.464100] i915_driver_irq_handler: 0= 1= > [ 3577.477383] i915_driver_irq_handler: 0= 1= > [ 3577.829016] i915_driver_irq_handler: 0=0002 1= > [ 3577.830093] i915_driver_irq_handler: 0=0002 1= > > I.e. the handler is called when IIR=0 and both pipe stats are 0. Oh sorry missed the PIPE*STAT, I thought it was IMR or something, I should have read more closely. So port hotplug is always reporting that port C has a hotplug interrupt though... If you write 0x3 back to it does the interrupt stop? -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120410/ad626351/attachment.pgp>
[Intel-gfx] [PATCH 1/4] drm/i915: handle input/output sdvo timings separately in mode_set
On Tue, 10 Apr 2012 18:36:49 +0200 Daniel Vetter wrote: > Well, neither do I have a clue about sdvo, but I think I somewhat > self-consistent explanation for what's going on. > > Sdvo seems to have two timings, one is the output timing which will be > sent over whatever is connected on the other side of the sdvo chip (panel, > hdmi screen, tv), the other is the input timing which will be generated by > the gmch pipe. It looks like sdvo is expected to scale between the two. Correct. And the SDVO encoder in the display engine will always run the clock at 100MHz+ and do data stuffing if the pixel rate is lower than that, but we need to set the right clock multiplier in that case (which we do). > To make things slightly more complicated, we have a bunch of special > cases: > - for lvds panel we always use a fixed output timing, namely > intel_sdvo->sdvo_lvds_fixed_mode, hence that special case. > - sdvo has an interface to generate a preferred input timing for a given > output timing. This is the confusing thing that I've tried to clear up > with the follow-on patches. > - a special requirement is that the input pixel clock needs to be between > 100MHz and 200MHz (likely to keep it within the electromechanical design > range of PCIe). Lower pixel clocks are doubled/quadrupled. > > The thing this patch tries to fix is that the pipe needs to be explicit > instructed to double/quadruple the pixels and needs the correspondingly > higher pixel clock, whereas the sdvo adaptor seems to do that itself and > needs the unadjusted pixel clock. Yeah that sounds right based on my reading of an old spec I have. > This patch tries to fix this mess by > - keeping the output mode timing in the unadjusted plain mode, safe for > the lvds case. > - store the input timing in the adjusted_mode with the adjusted pixel > clock. This way we don't need to frob around with the core crtc mode set > code. > - fixup the pixelclock when constructing the sdvo dtd timing struct. This > is why the first part of the patch is an integral part of the series. > - the is_tv special case can be dropped because input_dtd is equivalent to > adjusted_mode after these changes. Follow-up patches clear this up > further (by simply ripping out intel_sdvo->input_dtd because it's not > needed). > > Hopefully this clears things up a bit. Yep, thanks. Hopefully you'll get your SDVO spec access soon... -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120410/7ba762e3/attachment.pgp>
i915_driver_irq_handler: irq 42: nobody cared
On Tue, 10 Apr 2012 20:11:29 +0200
Jiri Slaby wrote:
> On 04/10/2012 06:26 PM, Jesse Barnes wrote:
> > So port hotplug is always reporting that port C has a hotplug
> > interrupt though... If you write 0x3 back to it does the interrupt
> > stop?
>
> I'm not sure I got it right. This doesn't help:
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1416,6 +1416,17 @@ static irqreturn_t
> i915_driver_irq_handler(DRM_IRQ_ARGS)
> iir = new_iir;
> }
>
> + if (ret == IRQ_NONE) {
> + u32 hp = I915_READ(PORT_HOTPLUG_STAT);
> + if (hp) {
> + I915_WRITE(PORT_HOTPLUG_STAT, hp);
> + I915_READ(PORT_HOTPLUG_STAT);
> + }
> +
> + if (printk_ratelimit())
> + printk(KERN_DEBUG "%s: %.8x\n", __func__, hp);
> +
> + }
>
> return ret;
> }
Yeah that looks right, you still get 0x300?
You could try masking hotplug interrupts altogether.
Also, just to sanity check things, can you look at the output of "lspci
-s 02.0 -vvv -xxx" and see if the "INTx" field is + or -? If it's +,
then the interrupt is definitely coming from an un-acked IRQ source on
the gfx device. If it's INTx-, it means something in one of the upper
MSI layers isn't getting handled right.
--
Jesse Barnes, Intel Open Source Technology Center
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120410/a477ec8a/attachment.pgp>
i915_driver_irq_handler: irq 42: nobody cared
On Tue, 10 Apr 2012 22:32:12 +0200 Daniel Vetter wrote: > On Tue, Apr 10, 2012 at 09:52:40PM +0200, Jiri Slaby wrote: > > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- > > SERR- > > > I tried 3.2 and 3.3. Although the spurious interrupts were always > > there, they occurred with frequency lower by a magnitude (15 vs. 300 > > after X starts). So I bisected that and it lead to a commit which > > fixes bad tiling for me: > > http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=for-jiri&id=79710e6ccabdac80c65cd13b944695ecc3e42a9d > > Pipelined fencing is pretty much just broken and we'll completely rip it > out in 3.5. Does this also happen with 3.4-rc2? Does INTx- stay that way? Or does it frequently read INTx+ if you sample it a lot? If it stays as INTx-, then something other than the GPU is getting stuck (though it's possible this could be related to pipelined fencing, if the fences are programmed to point at some funky memory space). -- Jesse Barnes, Intel Open Source Technology Center -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120410/1e0722c9/attachment.pgp>
i915_driver_irq_handler: irq 42: nobody cared
On Wed, 11 Apr 2012 08:29:22 +0200
Michel D?nzer wrote:
> On Die, 2012-04-10 at 11:34 -0700, Jesse Barnes wrote:
> > On Tue, 10 Apr 2012 20:11:29 +0200
> > Jiri Slaby wrote:
> >
> > > On 04/10/2012 06:26 PM, Jesse Barnes wrote:
> > > > So port hotplug is always reporting that port C has a hotplug
> > > > interrupt though... If you write 0x3 back to it does the interrupt
> > > > stop?
> > >
> > > I'm not sure I got it right. This doesn't help:
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1416,6 +1416,17 @@ static irqreturn_t
> > > i915_driver_irq_handler(DRM_IRQ_ARGS)
> > > iir = new_iir;
> > > }
> > >
> > > + if (ret == IRQ_NONE) {
> > > + u32 hp = I915_READ(PORT_HOTPLUG_STAT);
> > > + if (hp) {
> > > + I915_WRITE(PORT_HOTPLUG_STAT, hp);
> > > + I915_READ(PORT_HOTPLUG_STAT);
> > > + }
> > > +
> > > + if (printk_ratelimit())
> > > + printk(KERN_DEBUG "%s: %.8x\n", __func__, hp);
> > > +
> > > + }
> > >
> > > return ret;
> > > }
> >
> > Yeah that looks right, you still get 0x300?
>
> You said 'If you write 0x3 back' above, but this code writes 0x300.
> Which is right?
0x300 is right, the bits are status bits with write 1 to clear
semantics. But it looks like this one is just stuck high (probably
because port C isn't actually wired up fully).
--
Jesse Barnes, Intel Open Source Technology Center
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120411/a37832bb/attachment.pgp>
[RFC PATCH] drm: Add plane event
On Wed, 18 Apr 2012 16:58:36 +0200 Marcus Lorentzon wrote: > On 04/18/2012 04:26 PM, Ville Syrj?l? wrote: > Yes, from previous emails I have seen that we are quite aligned on the > single-atomic-modeset-with-properties version. > > Do you have any actual proposal for this? Like the API at least and some > comments on "the other limitations" you fix with it? > I only recall seeing Jesse's API proposal, but not yours, only some > ideas about a generic list of properties/values for modeset when I > proposed something similar. I've been talking with Ville in private about this a little. Doing things well means a few additional APIs and properties, but I don't think he has anything concrete yet (I expect he's hacking on something now and probably has it working, just not to his satisfaction :p). > I'm about to implement atomic modeset now one way or the other, so the > more proposals I have to choose from the better ;) > I found that the per-crtc modeset above covers my requirements, so I > might just take the easy route for now. But I welcome any work/proposal > on generic support for atomic modeset. I think Daniel summarized it well; it would be good to have an atomic mode set to change the whole device configuration atomically (including timings and other properties that involve global computation about memory bandwidth etc), and a separate ioctl for flipping new buffers onto one or more planes associated with a given pipe, along with ancillary data that may be needed (sprite position, z order, gamma, etc). This could easily spiral out of control though, given how poorly the existing KMS API expresses the variety of display controllers out there; hopefully we can keep things incremental. -- Jesse Barnes, Intel Open Source Technology Center
[RFC PATCH] drm: Add plane event
On Wed, 18 Apr 2012 17:55:15 +0200 Marcus Lorentzon wrote: > The async vs sync makes sense as reason for splitting them. My problem > lies somewhere in between sync modeset and async flip - async > crtc/plane/fbs modeset. > In Wayland and Android HW composer I need to asynchronously flip and do > crtc/plane modeset, but no connector/crtc modeset (so it is a fast > operation). Because I don't consider enable/disable/move a plane to be a > full synchronous modeset (same mode different fbs/planes). But I still > want the same async events to tell me the new plane setup is activated > at vsync. But as you say, maybe the biggest issue here is the "big drm > lock". So maybe user space would be ok to do this crtc-modeset in sync > mode, if it doesn't block other operations on other crtcs. But I would > prefer to be able to do the crtc modeset async so I don't have to have a > thread per crtc. Or am I missing the obvious solution to this? I don't think you're missing anything here; the obvious solution is the nuclear page flip. It's what I always envisioned would be needed once we had the basic sprite support in place. Basically we need a new page flip ioctl (which is async and gives you events) but that takes multiple planes as args, along with ancillary data. Neither setcrtc nor setplane are the right place to put this. Neither take all the info we want, and historically setcrtc didn't emit an event, so I didn't add it to setplane (it would be of limited usefulness anyway since we really want to flip primary + sprite at the same time). -- Jesse Barnes, Intel Open Source Technology Center
