[RFC] Standardize YUV support in the fbdev API
Oops. Nevermind, you already have looked at what ivtvfb does. -Andy
[RFC] Standardize YUV support in the fbdev API
On Wed, 2011-05-18 at 00:07 +0200, Laurent Pinchart wrote:
> Hi everybody,
>
> I need to implement support for a YUV frame buffer in an fbdev driver. As the
> fbdev API doesn't support this out of the box, I've spent a couple of days
> reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
> support to the API. I'd like to share my findings and thoughts, and hopefully
> receive some comments back.
I haven't looked at anything below, but I'll mention that the ivtv
driver presents an fbdev interface for the YUV output stream of the
CX23415.
It may be worth a look and asking Hans what are the short-comings.
-Andy
> The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
> will be used interchangeably in this e-mail. They all refer to the way pixels
> are stored in memory, including both the representation of a pixel as integer
> values and the layout of those integer values in memory.
>
>
> History and current situation
> -
>
> The fbdev API predates YUV frame buffers. In those old days developers only
> cared (and probably thought) about RGB frame buffers, and they developed an
> API that could express all kind of weird RGB layout in memory (such as R-
> GGG- for instance, I can't imagine hardware implementing that).
> This resulted in individual bit field description for the red, green, blue
> and
> alpha components:
>
> struct fb_bitfield {
> __u32 offset; /* beginning of bitfield*/
> __u32 length; /* length of bitfield */
> __u32 msb_right; /* != 0 : Most significant bit is */
>/* right */
> };
>
> Grayscale formats were pretty common, so a grayscale field tells color
> formats
> (grayscale == 0) from grayscale formats (grayscale != 0).
>
> People already realized that hardware developers were crazily inventive (the
> word to remember here is crazily), and that non-standard formats would be
> needed at some point. The fb_var_screeninfo ended up containing the following
> format-related fields.
>
> struct fb_var_screeninfo {
> ...
> __u32 bits_per_pixel; /* guess what */
> __u32 grayscale; /* != 0 Graylevels instead of colors */
>
> struct fb_bitfield red;/* bitfield in fb mem if true color, */
> struct fb_bitfield green; /* else only length is significant */
> struct fb_bitfield blue;
> struct fb_bitfield transp; /* transparency */
>
> __u32 nonstd; /* != 0 Non standard pixel format */
> ...
> };
>
> Two bits have been specified for the nonstd field:
>
> #define FB_NONSTD_HAM 1 /* Hold-And-Modify (HAM)*/
> #define FB_NONSTD_REV_PIX_IN_B 2 /* order of pixels in each byte is
> reversed
> */
>
> The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM
> mode[1]. I very much doubt that any new hardware will implement HAM mode (and
> I certainly hope none will).
>
> The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit,
> fillrect and copy area implementations, not directly by drivers.
>
> A couple of drivers hardcode the nonstd field to 1 for some reason. Those are
> video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and
> video/metronomefb.c (8bpp gray display).
>
> The following drivers use nonstd for various other (and sometimes weird)
> purposes:
>
> video/arkfb.c
> Used in 4bpp mode only, to control fb_setcolreg operation
> video/fsl-diu-fb.c
> Set var->nonstd bits into var->sync (why?)
> video/pxafb.c
> Encode frame buffer xpos and ypos in the nonstd field
> video/s3fb.c
> Used in 4bpp mode only, to control fb_setcolreg operation
> video/vga16fb.c
> When panning in non-8bpp, non-text mode, decrement xoffset
> Do some other weird stuff when not 0
> video/i810/i810_main.c
> Select direct color mode when set to 1 (truecolor otherwise)
>
> Fast forward a couple of years, hardware provides support for YUV frame
> buffers. Several drivers had to provide YUV format selection to applications,
> without any standard API to do so. All of them used the nonstd field for that
> purpose:
>
> media/video/ivtv/
> Enable YUV mode when set to 1
> video/pxafb.c
> Encode pixel format in the nonstd field
> video/sh_mobile_lcdfb.c
> If bpp == 12 and nonstd != 0, enable NV12 mode
> If bpp == 16 or bpp == 24, ?
> video/omap/omapfb_main.c
> Select direct color mode when set to 1 (depend on bpp otherwise)
> Used as a pixel format identifier (YUV422, YUV420 or YUY422)
> video/omap2/omapfb/omapfb-main.c
> Select direct color mode when set to 1 (depend on bpp otherwise)
> Used as a pixel format identifier
Re: [RFC] Standardize YUV support in the fbdev API
On Wed, 2011-05-18 at 00:07 +0200, Laurent Pinchart wrote:
Hi everybody,
I need to implement support for a YUV frame buffer in an fbdev driver. As the
fbdev API doesn't support this out of the box, I've spent a couple of days
reading fbdev (and KMS) code and thinking about how we could cleanly add YUV
support to the API. I'd like to share my findings and thoughts, and hopefully
receive some comments back.
I haven't looked at anything below, but I'll mention that the ivtv
driver presents an fbdev interface for the YUV output stream of the
CX23415.
It may be worth a look and asking Hans what are the short-comings.
-Andy
The terms 'format', 'pixel format', 'frame buffer format' and 'data format'
will be used interchangeably in this e-mail. They all refer to the way pixels
are stored in memory, including both the representation of a pixel as integer
values and the layout of those integer values in memory.
History and current situation
-
The fbdev API predates YUV frame buffers. In those old days developers only
cared (and probably thought) about RGB frame buffers, and they developed an
API that could express all kind of weird RGB layout in memory (such as R-
GGG- for instance, I can't imagine hardware implementing that).
This resulted in individual bit field description for the red, green, blue
and
alpha components:
struct fb_bitfield {
__u32 offset; /* beginning of bitfield*/
__u32 length; /* length of bitfield */
__u32 msb_right; /* != 0 : Most significant bit is */
/* right */
};
Grayscale formats were pretty common, so a grayscale field tells color
formats
(grayscale == 0) from grayscale formats (grayscale != 0).
People already realized that hardware developers were crazily inventive (the
word to remember here is crazily), and that non-standard formats would be
needed at some point. The fb_var_screeninfo ended up containing the following
format-related fields.
struct fb_var_screeninfo {
...
__u32 bits_per_pixel; /* guess what */
__u32 grayscale; /* != 0 Graylevels instead of colors */
struct fb_bitfield red;/* bitfield in fb mem if true color, */
struct fb_bitfield green; /* else only length is significant */
struct fb_bitfield blue;
struct fb_bitfield transp; /* transparency */
__u32 nonstd; /* != 0 Non standard pixel format */
...
};
Two bits have been specified for the nonstd field:
#define FB_NONSTD_HAM 1 /* Hold-And-Modify (HAM)*/
#define FB_NONSTD_REV_PIX_IN_B 2 /* order of pixels in each byte is
reversed
*/
The FB_NONSTD_HAM bit is used by the video/amifb.c driver only to enable HAM
mode[1]. I very much doubt that any new hardware will implement HAM mode (and
I certainly hope none will).
The FB_NONSTD_REV_PIX_IN_B is used in video/fb_draw.h by the generic bitblit,
fillrect and copy area implementations, not directly by drivers.
A couple of drivers hardcode the nonstd field to 1 for some reason. Those are
video/arcfb.c (1bpp gray display), video/hecubafb.c (1bpp gray display) and
video/metronomefb.c (8bpp gray display).
The following drivers use nonstd for various other (and sometimes weird)
purposes:
video/arkfb.c
Used in 4bpp mode only, to control fb_setcolreg operation
video/fsl-diu-fb.c
Set var-nonstd bits into var-sync (why?)
video/pxafb.c
Encode frame buffer xpos and ypos in the nonstd field
video/s3fb.c
Used in 4bpp mode only, to control fb_setcolreg operation
video/vga16fb.c
When panning in non-8bpp, non-text mode, decrement xoffset
Do some other weird stuff when not 0
video/i810/i810_main.c
Select direct color mode when set to 1 (truecolor otherwise)
Fast forward a couple of years, hardware provides support for YUV frame
buffers. Several drivers had to provide YUV format selection to applications,
without any standard API to do so. All of them used the nonstd field for that
purpose:
media/video/ivtv/
Enable YUV mode when set to 1
video/pxafb.c
Encode pixel format in the nonstd field
video/sh_mobile_lcdfb.c
If bpp == 12 and nonstd != 0, enable NV12 mode
If bpp == 16 or bpp == 24, ?
video/omap/omapfb_main.c
Select direct color mode when set to 1 (depend on bpp otherwise)
Used as a pixel format identifier (YUV422, YUV420 or YUY422)
video/omap2/omapfb/omapfb-main.c
Select direct color mode when set to 1 (depend on bpp otherwise)
Used as a pixel format identifier (YUV422 or YUY422)
Those drivers use the nonstd field in different, incompatible ways.
Other related APIs
Re: [RFC] Standardize YUV support in the fbdev API
Oops. Nevermind, you already have looked at what ivtvfb does. -Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH/RFC 0/5] HDMI driver for Samsung S5PV310 platform
On Wed, 2011-02-09 at 02:12 -0500, Alex Deucher wrote: > On Tue, Feb 8, 2011 at 5:47 PM, Andy Walls wrote: > > On Tue, 2011-02-08 at 10:28 -0500, Alex Deucher wrote: > >> On Tue, Feb 8, 2011 at 4:47 AM, Hans Verkuil wrote: > >> > Just two quick notes. I'll try to do a full review this weekend. > >> > > >> > On Tuesday, February 08, 2011 10:30:22 Tomasz Stanislawski wrote: > >> >> == > >> >> Introduction > >> >> == > >> >> > >> >> The purpose of this RFC is to discuss the driver for a TV output > >> >> interface > >> >> available in upcoming Samsung SoC. The HW is able to generate digital > >> >> and > >> >> analog signals. Current version of the driver supports only digital > >> >> output. > >> >> > >> >> Internally the driver uses videobuf2 framework, and CMA memory > >> >> allocator. > >> > Not > >> >> all of them are merged by now, but I decided to post the sources to > >> >> start > >> >> discussion driver's design. > > > >> > > >> > Cisco (i.e. a few colleagues and myself) are working on this. We hope to > >> > post > >> > an RFC by the end of this month. We also have a proposal for CEC support > >> > in > >> > the pipeline. > >> > >> Any reason to not use the drm kms APIs for modesetting, display > >> configuration, and hotplug support? We already have the > >> infrastructure in place for complex display configurations and > >> generating events for hotplug interrupts. It would seem to make more > >> sense to me to fix any deficiencies in the KMS APIs than to spin a new > >> API. Things like CEC would be a natural fit since a lot of desktop > >> GPUs support hdmi audio/3d/etc. and are already using kms. > >> > >> Alex > > > > I'll toss one out: lack of API documentation for driver or application > > developers to use. > > > > > > When I last looked at converting ivtvfb to use DRM, KMS, TTM, etc. (to > > possibly get rid of reliance on the ivtv X video driver > > http://dl.ivtvdriver.org/xf86-video-ivtv/ ), I found the documentation > > was really sparse. > > > > DRM had the most documentation under Documentation/DocBook/drm.tmpl, but > > the userland API wasn't fleshed out. GEM was talked about a bit in > > there as well, IIRC. > > > > TTM documentation was essentially non-existant. > > > > I can't find any KMS documentation either. > > > > I recall having to read much of the drm code, and having to look at the > > radeon driver, just to tease out what the DRM ioctls needed to do. > > > > Am I missing a Documentation source for the APIs? > > > > Documentation is somewhat sparse compared to some other APIs. Mostly > inline kerneldoc comments in the core functions. It would be nice to > improve things. The modesetting API is very similar to the xrandr > API in the xserver. > > At the moment a device specific surface manager (Xorg ddx, or some > other userspace lib) is required to use kms due to device specific > requirements with respect to memory management and alignment for > acceleration. The kms modesetting ioctls are common across all kms > drm drivers, but the memory management ioctls are device specific. > GEM itself is an Intel-specific memory manager, although radeon uses > similar ioctls. TTM is used internally by radeon, nouveau, and svga > for managing memory gpu accessible memory pools. Drivers are free to > use whatever memory manager they want; an existing one shared with a > v4l or platform driver, TTM, or something new. > There is no generic > userspace kms driver/lib although Dave and others have done some work > to support that, but it's really hard to make a generic interface > flexible enough to handle all the strange acceleration requirements of > GPUs. All of the above unfortunately says to me that the KMS API has a fairly tightly coupled set of userspace components, because userspace applications need have details about the specific underlying hardware embeeded in the application to effectively use the API. If so, that's not really conducive to getting application developers to write applications to the API, since applications will get tied to specific sets of hardware. Lack of documentation on the API for userpace application writers to use exacerbates that issue, as there are no clearly stated guarantees on device node conventions ioctl's
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Fri, 2010-09-24 at 15:12 -0400, Alex Deucher wrote: > On Wed, Sep 22, 2010 at 5:07 PM, Andy Walls > wrote: > > On Wed, 2010-09-22 at 09:33 -0400, Jon Smirl wrote: > >> On Wed, Sep 22, 2010 at 1:30 AM, Alex Deucher > >> wrote: > >> > On Tue, Sep 21, 2010 at 1:30 PM, Andy Walls > >> > wrote: > >> >> On Tue, 2010-09-21 at 00:26 -0400, Alex Deucher wrote: > >> >>> 2010/9/20 Andy Walls : > >> >>> > On Mon, 2010-09-20 at 20:29 +0200, Rafa? Mi?ecki wrote: > >> >>> >> 2010/9/20 Andy Walls : > > > >> >> The real problem to me is that the radeon and drm modules have a single, > >> >> standard way of dealing with EDID errors. However, EDID errors can > >> >> happen due to a number of different causes, some of which are external > >> >> (i.e. in the LCD or CRT monitor). Given that, there really is no "right > >> >> thing" the drivers can do without input from the user on what the policy > >> >> should be when a bad EDID is detected. > >> > The attached patch should fix up your board. Let me know if it works for you. That patch suppresses the setup of the HDMI and DVI connectors that don't exist, so the log spam from polling for EDID's is gone for me. [PATCH] drm/radeon/kms: add quirk for MSI K9A2GM motherboard Board has no digital connectors Reported-by: Andy Walls Tested-by: Andy Walls Signed-off-by: Alex Deucher Cc: stable at kernel.org Yes it works for me, but it's what I'd call a "point solution". There are still these which appear to be the exact same symptoms (perpetual EDID log spam) just for different hardware setups: https://bugs.freedesktop.org/show_bug.cgi?id=27708 https://partner-bugzilla.redhat.com/show_bug.cgi?id=610362 Here was a broader solution for a particular class of machines (Laptops with i915 chips with an LVDS with a broken EDID) exhibiting those same symptoms: https://patchwork.kernel.org/patch/83556/ Picking these bugs off one or two at a time, as users report the same symptoms over an over again, is really a waste of users' time. Users wait while their report is queued, a custom kernel patch developed, and a kernel patch makes it through their distro to them. On a different subject, with your patch applied, I'm now seeing a new error message in my log that I have not seen before: failed to evaluate ATIF got AE_BAD_PARAMETER Here's the dmesg from drm: [drm] Initialized drm 1.1.0 20060810 [drm] radeon defaulting to kernel modesetting. [drm] radeon kernel modesetting enabled. radeon :01:05.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18 [drm] initializing kernel modesetting (RS740 0x1002:0x796E). [drm] register mmio base: 0xFE7F [drm] register mmio size: 65536 ATOM BIOS: ATI radeon :01:05.0: VRAM: 128M 0x7800 - 0x7FFF (128M used) radeon :01:05.0: GTT: 512M 0x8000 - 0x9FFF [drm] radeon: irq initialized. [drm] Detected VRAM RAM=128M, BAR=128M [drm] RAM width 128bits DDR [TTM] Zone kernel: Available graphics memory: 963022 kiB. [TTM] Initializing pool allocator. [drm] radeon: 128M of VRAM memory ready [drm] radeon: 512M of GTT memory ready. [drm] GART: num cpu pages 131072, num gpu pages 131072 [drm] radeon: 1 quad pipes, 1 z pipes initialized. [drm] Loading RS690/RS740 Microcode [drm] radeon: ring at 0x8000 [drm] ring test succeeded in 1 usecs [drm] radeon: ib pool ready. [drm] ib test succeeded in 0 usecs [drm] Enabling audio support failed to evaluate ATIF got AE_BAD_PARAMETER [drm] Default TV standard: NTSC [drm] Radeon Display Connectors [drm] Connector 0: [drm] VGA [drm] DDC: 0x7e50 0x7e40 0x7e54 0x7e44 0x7e58 0x7e48 0x7e5c 0x7e4c [drm] Encoders: [drm] CRT1: INTERNAL_KLDSCP_DAC1 [drm] fb mappable at 0xD804 [drm] vram apper at 0xD800 [drm] size 4325376 [drm] fb depth is 24 [drm]pitch is 5632 fbcon: radeondrmfb (fb0) is primary device Console: switching to colour frame buffer device 170x48 fb0: radeondrmfb frame buffer device drm: registered panic notifier Slow work thread pool: Starting up Slow work thread pool: Ready [drm] Initialized radeon 2.6.0 20080528 for :01:05.0 on minor 0 By inspection of the code, it looks like the handle being passed into radeon_atif_call() by radeon_acpi_init() may be bad. I'm not sure why that would be though. I won't have time until Thursday to run it down any further. Regards, Andy > Thanks, > > Alex
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 09:33 -0400, Jon Smirl wrote:
> On Wed, Sep 22, 2010 at 1:30 AM, Alex Deucher
> wrote:
> > On Tue, Sep 21, 2010 at 1:30 PM, Andy Walls
> > wrote:
> >> On Tue, 2010-09-21 at 00:26 -0400, Alex Deucher wrote:
> >>> 2010/9/20 Andy Walls :
> >>> > On Mon, 2010-09-20 at 20:29 +0200, Rafa? Mi?ecki wrote:
> >>> >> 2010/9/20 Andy Walls :
> >> The real problem to me is that the radeon and drm modules have a single,
> >> standard way of dealing with EDID errors. However, EDID errors can
> >> happen due to a number of different causes, some of which are external
> >> (i.e. in the LCD or CRT monitor). Given that, there really is no "right
> >> thing" the drivers can do without input from the user on what the policy
> >> should be when a bad EDID is detected.
>
> Andy, this sure looks like a broken VBIOS to me.
Well sure.
But that problem causes other problems in error handling code paths to
surface. It also brings to light that there are some cases that are
undecidable, or not worth the effort, for the error handling code paths
on what the proper action should be.
> First thing would be
> to update your VBIOS if possible to get a correct table for your
> hardware.
Um, no.
I will not risk taking an operational machine down due to flash write
failure, however small the probability, due to the high impact.
The reward of shutting up kernel error messages, is not worth the risk.
> Second would be to add a quirk in the kernel.
I have expressed my thoughts on quirks in a previous post.
> There are lots of cases where the kernel does odd things when the BIOS
> feeds it bad information. Do we really want hundreds of switches in
> sysfs allowing adjustments for broken BIOS features?
I see very little downside in giving the user more control over his
system. A thousand knobs and switches are worth it for the user, for
the one switch that is there when the user needs it to solve a problem.
To dump my VBIOS ROM for Alex, I could have hacked up the radeon driver
to dump the ROM. That would have consumed a lot of time. Luckily for
me, there was a switch to turn on the ROM and dump it:
# echo 1 > /sys/class/drm/card0/device/rom
# dd if=/sys/class/drm/card0/device/rom of=msi7302igprom.bin
# echo 0 > /sys/class/drm/card0/device/rom
I never used it before and will likely never use it again. But when I
had a problem I needed to solve, its availability made the solution
simple and efficient. Time to accomplish tasks is my scarcest resource;
time efficiency is very important to me.
The only downside to hundreds of switches and control knobs I can really
think of is possibly complexity for the end user. But it turns out,
that ignoring the available controls, or ignoring large subsets of the
available controls, is how people are going to deal with that
complexity. Heck, I ignore most of sysfs almost all the time. I also
ignore almost every module option available. My system runs fine
without me caring about a majority of the existing switches.
BTW, we already have thousands of switches and controls for kernel
internals in linux without sysfs and ioctl()'s:
$ find /lib/modules/`uname -r` -name "*.ko" -exec modinfo {} \; | grep '^parm:'
| wc -l
3387
Why do we have that many? They are low cost in complexity, as they can
easily be ignored. They are high value in utility, as they give the
user control over his system to deal with unusual circumstances.
> We already have
> the quirk scheme for addressing this.
>
> A simpler solution for reducing the log spam would be to only report
> the error once, instead of every 10 seconds. The driver could remember
> it has made the error report and then log another message later if the
> error was cleared.
My sysfs implementation was only 69 changed lines in one file:
drivers/gpu/drm/drm_sysfs.c | 69 +++
I doubt a solution to add logic to the error paths, to try and divine
all the sources of EDID errors by saving state and applying rules to
take the correct action, is going to be less change than that. I know
more than one file will have to change.
Regards,
Andy
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 08:31 -0400, Andy Walls wrote: > On Wed, 2010-09-22 at 01:30 -0400, Alex Deucher wrote: > > It appears to be incorrect. If you send me a copy of the vbios, I can > > easily add a quirk. I suspect your board oem may offer several boards > > with different output configurations and neglected to update the table > > in some configurations. > > Do you have a pointer to a set of steps to follow for extracting that > vbios information? Never mind, I figured it out. # echo 1 > /sys/class/drm/card0/device/rom # dd if=/sys/class/drm/card0/device/rom of=msi7302igprom.bin # echo 0 > /sys/class/drm/card0/device/rom I will send it via private email. Regards, Andy
[PATCH v2 2/2] Doc/ABI: sysfs-drm initial document; "polled" entry
On Wed, 2010-09-22 at 08:06 -0700, Greg KH wrote: > On Tue, Sep 21, 2010 at 11:20:22PM -0400, Andy Walls wrote: > > +Where: /sys/devices/<...>/drm/cardN/cardN-C-M/polled > > + For N a decimal system graphics adapter number: 0, 1, 2, ... > > + For C a connector type name (including spaces) from the set: > > Spaces? Really? Yeah, I know it will work just fine, but why go out of > your way to make it hard for people? Not my fault. It was like that when I got here. $ ls -d /sys/devices/pci\:00/\:00\:01.0/\:01\:05.0/drm//card0/card0* /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-DVI-D-1 /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-HDMI Type A-1 /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-VGA-1 I only added one entry, "polled", in those existing directories. BTW, Jon Smirl, IBM Corp, and you have Copyright credit in the drm_sysfs.c file that creates those directories with spaces. ;) I'll also gripe that PCI's colons are pretty annoying on the command line too, since they have to be escaped as well. > > + HDMI Type A > > + HDMI Type B > > + TV > > + Embedded DisplayPort > > You could always just use a '_' instead of a space for those that need > it. A trivial patch here would do that: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob;f=drivers/gpu/drm/drm_crtc.c;h=37e0b4fa482a810afc9eded6fda136a90bcc5cc0;hb=refs/heads/drm-fixes#l146 but I have no idea what that might break. Regards, Andy
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 11:44 +0200, Florian Mickler wrote: > [cc'd chris wilson] Oops. Thanks! > Hi Andy! > > On Mon, 20 Sep 2010 19:02:30 -0400 > Andy Walls wrote: > > > BTW, I found that Chris Wilson recently committed a change to inhibit > > all drm connector polling globally for a different reason: > > > > http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commit;h=e58f637bb96d5a0ae0919b9998b891d1ba7e47c9 > > > > That commit message shows a case where the driver decides polling needs > > to happen, but the human knows differently and manual control over > > connector polling mitigates the problem. > > > On Mon, 20 Sep 2010 17:11:48 -0400 > Andy Walls wrote: > > > On Mon, 2010-09-20 at 11:52 -0700, Greg KH wrote: > > > On Mon, Sep 20, 2010 at 08:59:00AM -0400, Andy Walls wrote: > > > > > > This change allows the root user to disable (and re-enable) DRM KMS > > > > connector polling on a per connector basis via sysfs, like so: > > > > > > > > # cat /sys/class/drm/card0/card0-DVI-D-1/polled > > > > [hotplug_detectable] connect disconnect > > > > > You are adding a sysfs file, yet you forgot to add a file in > > > Documentation/ABI. Please fix that and resend the patch. > > > > Oops, process failure, sorry. > > Will do. > > > > Regards, > > Andy > > > > I thought sysfs files should be one thing per file... so, maybe > card0-DVI-D-1/link_status and card0-DVI-D-1/hotplug_detectable with 0/1 > content would be easier to manipulate and parse? If precedent matters, the sysfs nodes for setting the IO scheduler the active trigger function on an LED the active IR remote control protocols all use the same sort of paradigm as I did. The IO scheduler and LED trigger ones allow the user to set 1 of N choices. The IR protocol one allows the user to set M of N choices. They all uses brackets to differentiate [set] vs unset. > I have to defer to the drm maintainers for the usecases. But how is > having a monitor with a broken edid handled right now? While the output > is connected and used, it probably just stops polling? Not from what I can see. I could very well be wrong on that, so please someone double check me. This error message sequence, and from what I saw in the code, indicates to me that the gripe for a constantly bad EDID will never end: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: radeon :01:05.0: DVI-D-1: EDID invalid. Sep 22 07:46:13 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1: probed a monitor but no|invalid EDID Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: radeon :01:05.0: DVI-D-1: EDID invalid. Sep 22 07:46:13 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1: probed a monitor but no|invalid EDID This time around we "probed a monitor but no|invalid EDID" so lets do it again later, and maybe we'll get a different result. That's legitimate to do once or twice because of transient conditions: one may get a bad EDID due to monitor power on/off or cable connect/disconnect. To keep doing it for persistent error conditions, when the user fully understands the source of the error and has assessed the impact as inconsequential, is annoying. By now, I guess everyone can tell at least I'm annoyed by it. :) Regards, Andy
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 01:30 -0400, Alex Deucher wrote: > On Tue, Sep 21, 2010 at 1:30 PM, Andy Walls > wrote: > > In a larger sense it's about user policy for error reporting by the drm > > and radeon drivers, and error response by both the drivers and the user. > > > > In the face of EDID errors, a user may want to take the following > > actions: > > > >ignore the errors > >supress the errors > >continue to monitor the errors for a time > >replace cables > >replace graphics adapter > >replace monitor > >report a bug > > > > All of those end user actions are possible right now, except the one to > > supress the errors (and the I2C transactions associated with them). > > > > A monitor, cable, and graphics adapter set that is currently > > experiencing EDID errors, may be otherwise working just fine. > > > > "Because we want to ensure people report bugs," does not seem like a > > good reason to prevent a user from turning off bogus error messages, for > > an otherwise working monitor that is providing broken EDID data. What > > is the resolution of a bug report for a monitor providing bad EDID data > > going to look like? > > I wasn't talking about the broken EDID messages, I was talking about > the bogus connector table entries on your board. Those should be > properly quirked in the driver in which case, which would avoid the > problem all together in your case. The problem with in-kernel quirk databases are - you'll always end up lagging behind what's being fielded, - the number of stale entries will grow over the years, - the scope of the fix for a single quirk entry for the total installed user base is small; each quirk table entry itself has little value - quirks for working around unreliable attributes still need a reliable system identification method (no big deal for PCI) - fixing by quirk is a reactionary mode for kernel developers (it's also job security, so sysadmins are stuck coming back for help) - it increases time to resolve problems for end users. I don't think it's a good to plan to continually react to manufacturers that have cycle times of 6 months to a year. Quirk tables should be a solution of last resort, when there's nothing else that can be done, IMO. I understood. A quirk database solves the immediate problem for me and for owners of that MSI motherboard. My current problem also highlights undesirable behavior that will occur with broken EDIDs. This undesirable behavior will likely resurface in another bug report. > >> or does the board have a bug in the connector table? > > > > I have not dug into the BIOS connector table bits and bytes to verify, > > but I'm assuming the connector table is incorrect. > > > > It appears to be incorrect. If you send me a copy of the vbios, I can > easily add a quirk. I suspect your board oem may offer several boards > with different output configurations and neglected to update the table > in some configurations. Do you have a pointer to a set of steps to follow for extracting that vbios information? > >> What > >> physical connectors does the board have what does the driver report in > >> dmesg? > > > > The board only has a physical VGA connector. The graphics adapter is an > > IGP on an MSI K9A2GM V2/V3 series motherboard (MS-7302). Here are the > > PCI IDs on the graphics adapter: > > > > 01:05.0 VGA compatible controller [0300]: > >ATI Technologies Inc Radeon 2100 [1002:796e] (prog-if 00 [VGA > > controller]) > >Subsystem: Micro-Star International Co., Ltd. Device [1462:7302] > > > > > > Below is the dmesg output from the unmodified drm and radeon modules > > which includes samples of drm module log spam for the first 30 seconds > > or so after boot. > > [drm] Radeon Display Connectors > > [drm] Connector 0: > > [drm] VGA > > [drm] DDC: 0x7e50 0x7e40 0x7e54 0x7e44 0x7e58 0x7e48 0x7e5c 0x7e4c > > [drm] Encoders: > > [drm] CRT1: INTERNAL_KLDSCP_DAC1 > > [drm] Connector 1: > > [drm] DVI-D > > [drm] HPD2 > > [drm] DDC: 0x7e40 0x7e60 0x7e44 0x7e64 0x7e48 0x7e68 0x7e4c 0x7e6c > > [drm] Encoders: > > [drm] DFP2: INTERNAL_DDI > > [drm] Connector 2: > > [drm] HDMI-A > > [drm] DDC: 0x7e40 0x7e50 0x7e44 0x7e54 0x7e48 0x7e58 0x7e4c 0x7e5c > > [drm] Encoders: > > [drm] DFP3: INTERNAL_LVTM1 > > FWIW, it's actually the HDMI connector that is generating the errors > due to the lack of a hpd pin assignment. Are you sure? My /var/log/mes
[PATCH v2 2/2] Doc/ABI: sysfs-drm initial document; "polled" entry
This is the initial addition of documentation for the drm module's sysfs entries. It provides a drm sysfs entries overview, and a detailed description of the new drm per output connector "polled" entry in sysfs. Signed-of-by: Andy Walls diff --git a/Documentation/ABI/testing/sysfs-drm b/Documentation/ABI/testing/sysfs-drm new file mode 100644 index 000..18a017a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-drm @@ -0,0 +1,208 @@ +Direct Rendering Infrastructure (DRI) Direct Rendering Manager (drm) module +and Translation Table Manager (ttm) module sysfs entries + +Example class view showing a single graphics adapter with three output +connectors, a special control device node for the graphics adapter, and the +Translation Table Manager (ttm) graphics memory manager virtual device: + +/sys/class/drm/ +??? card0 -> ../../devices/<...>/drm/card0 +??? card0-DVI-D-1 -> ../../devices/<...>/drm/card0/card0-DVI-D-1 +??? card0-HDMI Type A-1 -> ../../devices/<...>/drm/card0/card0-HDMI Type A-1 +??? card0-VGA-1 -> ../../devices/<...>/drm/card0/card0-VGA-1 +??? controlD64 -> ../../devices/<...>/drm/controlD64 +??? ttm -> ../../devices/virtual/drm/ttm +??? version + +6 directories, 1 file + + +Corresponding DRI device nodes for the example graphics adapter, with +additional ACL properties on card0 granting rw perms to the X/console user: + +crw-rw+ 1 root video 226, 0 2010-09-21 20:52 /dev/dri/card0 +crw-rw-rw-. 1 root video 226, 64 2010-09-21 20:51 /dev/dri/controlD64 + + +Example physical device view for a single graphics adapter with three output +connectors and a special control device node for the graphics adapter: + +/sys/devices/<...>/drm +??? card0 +? ??? card0-DVI-D-1 +? ? ??? device -> ../../card0 +? ? ??? dpms +? ? ??? edid +? ? ??? enabled +? ? ??? modes +? ? ??? polled +? ? ??? power +? ? ? ??? control +? ? ? ??? runtime_active_time +? ? ? ??? runtime_status +? ? ? ??? runtime_suspended_time +? ? ? ??? wakeup +? ? ? ??? wakeup_count +? ? ??? status +? ? ??? subsystem -> ../../../../../../../class/drm +? ? ??? uevent +? ??? card0-HDMI Type A-1 +? ? ??? device -> ../../card0 +? ? ??? dpms +? ? ??? edid +? ? ??? enabled +? ? ??? modes +? ? ??? polled +? ? ??? power +? ? ? ??? control +? ? ? ??? runtime_active_time +? ? ? ??? runtime_status +? ? ? ??? runtime_suspended_time +? ? ? ??? wakeup +? ? ? ??? wakeup_count +? ? ??? status +? ? ??? subsystem -> ../../../../../../../class/drm +? ? ??? uevent +? ??? card0-VGA-1 +? ? ??? device -> ../../card0 +? ? ??? dpms +? ? ??? edid +? ? ??? enabled +? ? ??? modes +? ? ??? polled +? ? ??? power +? ? ? ??? control +? ? ? ??? runtime_active_time +? ? ? ??? runtime_status +? ? ? ??? runtime_suspended_time +? ? ? ??? wakeup +? ? ? ??? wakeup_count +? ? ??? status +? ? ??? subsystem -> ../../../../../../../class/drm +? ? ??? uevent +? ??? dev +? ??? device -> ../../../:01:05.0 +? ??? power +? ? ??? control +? ? ??? runtime_active_time +? ? ??? runtime_status +? ? ??? runtime_suspended_time +? ? ??? wakeup +? ? ??? wakeup_count +? ??? subsystem -> ../../../../../../class/drm +? ??? uevent +??? controlD64 +??? dev +??? device -> ../../../:01:05.0 +??? power +? ??? control +? ??? runtime_active_time +? ??? runtime_status +? ??? runtime_suspended_time +? ??? wakeup +? ??? wakeup_count +??? subsystem -> ../../../../../../class/drm +??? uevent + +20 directories, 55 files + + +Example virtual device view view showing the Translation Table Manager (ttm) +graphics memory manager virtual device: + +/sys/devices/virtual/drm +??? ttm +??? buffer_objects +? ??? bo_count +??? memory_accounting +? ??? kernel +? ? ??? available_memory +? ? ??? emergency_memory +? ? ??? swap_limit +? ? ??? used_memory +? ? ??? zone_memory +? ??? pool +? ??? pool_allocation_size +? ??? pool_max_size +? ??? pool_small_allocation +??? power +? ??? control +? ??? runtime_active_time +? ??? runtime_status +? ??? runtime_suspended_time +? ??? wakeup +?
[PATCH v2 1/2] drm/sysfs: Provide per connector control of DRM KMS polling
DRM KMS polling of connections providing errant EDID responses, or
polling of "connectors" that have chips responding on DDC I2C bus
address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM,
will create perpetual noise in dmesg and the system log every 10
seconds. Currently the user has apparently little recourse to silence
these messages aside from replacing the offending cable, monitor, or
graphics adapter. That recourse is impossible for an unused DVI-D
"connector" of an internal graphics processor on a motherboard that
provides no physical DVI-D connector.
This change allows the root user to disable (and re-enable) DRM KMS
connector polling on a per connector basis via sysfs, like so:
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] connect disconnect
# echo > /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
# echo " connect hotplug_detectable " > \
/sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] [connect] disconnect
# echo > /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
with the enabled poll types for the connector denoted in brackets: [].
This allows the root user to silence DRM KMS log spam for locally known
uncorrectable conditions.
Signed-off-by Andy Walls
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 86118a7..8e0807d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -318,11 +318,80 @@ static ssize_t select_subconnector_show(struct device
*device,
drm_get_dvi_i_select_name((int)subconnector));
}
+static const struct {
+ uint8_t mask;
+ const char *name;
+} polled_bit_names[] = {
+ { DRM_CONNECTOR_POLL_HPD,"hotplug_detectable" },
+ { DRM_CONNECTOR_POLL_CONNECT,"connect"},
+ { DRM_CONNECTOR_POLL_DISCONNECT, "disconnect" },
+};
+
+/*
+ * Return the decoded contents of connector->polled, using the names of the
+ * all the bit masks. Bits that are set, have their names enclosed in
brackets.
+ */
+static ssize_t polled_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ char *tmp;
+ int i;
+
+ tmp = buf;
+ for (i = 0; i < ARRAY_SIZE(polled_bit_names); i++) {
+ if (connector->polled & polled_bit_names[i].mask)
+ tmp += sprintf(tmp, "[%s] ", polled_bit_names[i].name);
+ else
+ tmp += sprintf(tmp, "%s ", polled_bit_names[i].name);
+ }
+
+ if (tmp != buf)
+ *(tmp - 1) = '\n';
+ return tmp - buf;
+}
+
+/*
+ * Change the state of connector->polled, given input bit-mask name-strings
+ * that are separated by space or newline.
+ */
+static ssize_t polled_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ const char *tmp;
+ int i;
+ uint8_t polled;
+
+ /* Polling of this connector will cease, if no match is made below */
+ polled = 0;
+
+ /* Incrementally split and parse the input */
+ while ((tmp = strsep((char **) , " \n")) != NULL) {
+
+ /* Don't waste effort on multiple adjacent separators */
+ if (*tmp == '\0')
+ continue;
+
+ /* Check for a match with a connector poll type name */
+ for (i = 0; i < ARRAY_SIZE(polled_bit_names); i++) {
+ if (!strncasecmp(tmp, polled_bit_names[i].name,
+strlen(polled_bit_names[i].name))) {
+ polled |= polled_bit_names[i].mask;
+ break;
+ }
+ }
+ }
+ connector->polled = polled;
+ return count;
+}
+
static struct device_attribute connector_attrs[] = {
__ATTR_RO(status),
__ATTR_RO(enabled),
__ATTR_RO(dpms),
__ATTR_RO(modes),
+ __ATTR(polled, 0644, polled_show, polled_store),
};
/* These attributes are for both DVI-I connectors and all types of tv-out. */
[PATCH v2 0/2] drm/sysfs: Provide per connector control of DRM KMS polling
Per Greg's request, here is a resend of my previous patch to add sysfs entries to allow manual override of DRM KMS connector polling actions, with accompanying documentation. Only documentation has been added. No code has changed from the previous patch submission. diffstat: Documentation/ABI/testing/sysfs-drm | 208 drivers/gpu/drm/drm_sysfs.c | 69 +++ 2 files changed, 277 insertions(+) Regards, Andy
Re: [PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 01:30 -0400, Alex Deucher wrote: On Tue, Sep 21, 2010 at 1:30 PM, Andy Walls awa...@md.metrocast.net wrote: In a larger sense it's about user policy for error reporting by the drm and radeon drivers, and error response by both the drivers and the user. In the face of EDID errors, a user may want to take the following actions: ignore the errors supress the errors continue to monitor the errors for a time replace cables replace graphics adapter replace monitor report a bug All of those end user actions are possible right now, except the one to supress the errors (and the I2C transactions associated with them). A monitor, cable, and graphics adapter set that is currently experiencing EDID errors, may be otherwise working just fine. Because we want to ensure people report bugs, does not seem like a good reason to prevent a user from turning off bogus error messages, for an otherwise working monitor that is providing broken EDID data. What is the resolution of a bug report for a monitor providing bad EDID data going to look like? I wasn't talking about the broken EDID messages, I was talking about the bogus connector table entries on your board. Those should be properly quirked in the driver in which case, which would avoid the problem all together in your case. The problem with in-kernel quirk databases are - you'll always end up lagging behind what's being fielded, - the number of stale entries will grow over the years, - the scope of the fix for a single quirk entry for the total installed user base is small; each quirk table entry itself has little value - quirks for working around unreliable attributes still need a reliable system identification method (no big deal for PCI) - fixing by quirk is a reactionary mode for kernel developers (it's also job security, so sysadmins are stuck coming back for help) - it increases time to resolve problems for end users. I don't think it's a good to plan to continually react to manufacturers that have cycle times of 6 months to a year. Quirk tables should be a solution of last resort, when there's nothing else that can be done, IMO. I understood. A quirk database solves the immediate problem for me and for owners of that MSI motherboard. My current problem also highlights undesirable behavior that will occur with broken EDIDs. This undesirable behavior will likely resurface in another bug report. or does the board have a bug in the connector table? I have not dug into the BIOS connector table bits and bytes to verify, but I'm assuming the connector table is incorrect. It appears to be incorrect. If you send me a copy of the vbios, I can easily add a quirk. I suspect your board oem may offer several boards with different output configurations and neglected to update the table in some configurations. Do you have a pointer to a set of steps to follow for extracting that vbios information? What physical connectors does the board have what does the driver report in dmesg? The board only has a physical VGA connector. The graphics adapter is an IGP on an MSI K9A2GM V2/V3 series motherboard (MS-7302). Here are the PCI IDs on the graphics adapter: 01:05.0 VGA compatible controller [0300]: ATI Technologies Inc Radeon 2100 [1002:796e] (prog-if 00 [VGA controller]) Subsystem: Micro-Star International Co., Ltd. Device [1462:7302] Below is the dmesg output from the unmodified drm and radeon modules which includes samples of drm module log spam for the first 30 seconds or so after boot. [drm] Radeon Display Connectors [drm] Connector 0: [drm] VGA [drm] DDC: 0x7e50 0x7e40 0x7e54 0x7e44 0x7e58 0x7e48 0x7e5c 0x7e4c [drm] Encoders: [drm] CRT1: INTERNAL_KLDSCP_DAC1 [drm] Connector 1: [drm] DVI-D [drm] HPD2 [drm] DDC: 0x7e40 0x7e60 0x7e44 0x7e64 0x7e48 0x7e68 0x7e4c 0x7e6c [drm] Encoders: [drm] DFP2: INTERNAL_DDI [drm] Connector 2: [drm] HDMI-A [drm] DDC: 0x7e40 0x7e50 0x7e44 0x7e54 0x7e48 0x7e58 0x7e4c 0x7e5c [drm] Encoders: [drm] DFP3: INTERNAL_LVTM1 FWIW, it's actually the HDMI connector that is generating the errors due to the lack of a hpd pin assignment. Are you sure? My /var/log/messages spam insists it is DVI-D-1 (hex dumps omitted): Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: radeon :01:05.0: DVI-D-1: EDID invalid. Sep 22 07:46:13 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1: probed a monitor but no|invalid EDID Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw
Re: [PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 11:44 +0200, Florian Mickler wrote: [cc'd chris wilson] Oops. Thanks! Hi Andy! On Mon, 20 Sep 2010 19:02:30 -0400 Andy Walls awa...@md.metrocast.net wrote: BTW, I found that Chris Wilson recently committed a change to inhibit all drm connector polling globally for a different reason: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commit;h=e58f637bb96d5a0ae0919b9998b891d1ba7e47c9 That commit message shows a case where the driver decides polling needs to happen, but the human knows differently and manual control over connector polling mitigates the problem. On Mon, 20 Sep 2010 17:11:48 -0400 Andy Walls awa...@md.metrocast.net wrote: On Mon, 2010-09-20 at 11:52 -0700, Greg KH wrote: On Mon, Sep 20, 2010 at 08:59:00AM -0400, Andy Walls wrote: This change allows the root user to disable (and re-enable) DRM KMS connector polling on a per connector basis via sysfs, like so: # cat /sys/class/drm/card0/card0-DVI-D-1/polled [hotplug_detectable] connect disconnect You are adding a sysfs file, yet you forgot to add a file in Documentation/ABI. Please fix that and resend the patch. Oops, process failure, sorry. Will do. Regards, Andy I thought sysfs files should be one thing per file... so, maybe card0-DVI-D-1/link_status and card0-DVI-D-1/hotplug_detectable with 0/1 content would be easier to manipulate and parse? If precedent matters, the sysfs nodes for setting the IO scheduler the active trigger function on an LED the active IR remote control protocols all use the same sort of paradigm as I did. The IO scheduler and LED trigger ones allow the user to set 1 of N choices. The IR protocol one allows the user to set M of N choices. They all uses brackets to differentiate [set] vs unset. I have to defer to the drm maintainers for the usecases. But how is having a monitor with a broken edid handled right now? While the output is connected and used, it probably just stops polling? Not from what I can see. I could very well be wrong on that, so please someone double check me. This error message sequence, and from what I saw in the code, indicates to me that the gripe for a constantly bad EDID will never end: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: radeon :01:05.0: DVI-D-1: EDID invalid. Sep 22 07:46:13 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1: probed a monitor but no|invalid EDID Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID: Sep 22 07:46:13 morgan kernel: radeon :01:05.0: DVI-D-1: EDID invalid. Sep 22 07:46:13 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1: probed a monitor but no|invalid EDID This time around we probed a monitor but no|invalid EDID so lets do it again later, and maybe we'll get a different result. That's legitimate to do once or twice because of transient conditions: one may get a bad EDID due to monitor power on/off or cable connect/disconnect. To keep doing it for persistent error conditions, when the user fully understands the source of the error and has assessed the impact as inconsequential, is annoying. By now, I guess everyone can tell at least I'm annoyed by it. :) Regards, Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] Doc/ABI: sysfs-drm initial document; polled entry
On Wed, 2010-09-22 at 08:06 -0700, Greg KH wrote: On Tue, Sep 21, 2010 at 11:20:22PM -0400, Andy Walls wrote: +Where: /sys/devices/.../drm/cardN/cardN-C-M/polled + For N a decimal system graphics adapter number: 0, 1, 2, ... + For C a connector type name (including spaces) from the set: Spaces? Really? Yeah, I know it will work just fine, but why go out of your way to make it hard for people? Not my fault. It was like that when I got here. $ ls -d /sys/devices/pci\:00/\:00\:01.0/\:01\:05.0/drm//card0/card0* /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-DVI-D-1 /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-HDMI Type A-1 /sys/devices/pci:00/:00:01.0/:01:05.0/drm//card0/card0-VGA-1 I only added one entry, polled, in those existing directories. BTW, Jon Smirl, IBM Corp, and you have Copyright credit in the drm_sysfs.c file that creates those directories with spaces. ;) I'll also gripe that PCI's colons are pretty annoying on the command line too, since they have to be escaped as well. + HDMI Type A + HDMI Type B + TV + Embedded DisplayPort You could always just use a '_' instead of a space for those that need it. A trivial patch here would do that: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob;f=drivers/gpu/drm/drm_crtc.c;h=37e0b4fa482a810afc9eded6fda136a90bcc5cc0;hb=refs/heads/drm-fixes#l146 but I have no idea what that might break. Regards, Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Wed, 2010-09-22 at 08:31 -0400, Andy Walls wrote: On Wed, 2010-09-22 at 01:30 -0400, Alex Deucher wrote: It appears to be incorrect. If you send me a copy of the vbios, I can easily add a quirk. I suspect your board oem may offer several boards with different output configurations and neglected to update the table in some configurations. Do you have a pointer to a set of steps to follow for extracting that vbios information? Never mind, I figured it out. # echo 1 /sys/class/drm/card0/device/rom # dd if=/sys/class/drm/card0/device/rom of=msi7302igprom.bin # echo 0 /sys/class/drm/card0/device/rom I will send it via private email. Regards, Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Tue, 2010-09-21 at 00:26 -0400, Alex Deucher wrote: > 2010/9/20 Andy Walls : > > On Mon, 2010-09-20 at 20:29 +0200, Rafa? Mi?ecki wrote: > >> 2010/9/20 Andy Walls : > >> > DRM KMS polling of connections providing errant EDID responses, or > >> > polling of "connectors" that have chips responding on DDC I2C bus > >> > address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM, > >> > will create perpetual noise in dmesg and the system log every 10 > >> > seconds. Currently the user has apparently little recourse to silence > >> > these messages aside from replacing the offending cable, monitor, or > >> > graphics adapter. That recourse is impossible for an unused DVI-D > >> > "connector" of an internal graphics processor on a motherboard that > >> > provides no physical DVI-D connector. > >> > > >> > This change allows the root user to disable (and re-enable) DRM KMS > >> > connector polling on a per connector basis via sysfs, like so: > >> > >> Huh, I didn't imagine solution of this issue that way. > > > > The problems with my initial patch, and related approaches, were: > > > > 1. They didn't get rid of all the spam in the logs, just the most > > verbose portion of it. > > > > 2. They didn't deal with the root cause of the log spam and all the > > related, but unneeded, I/O and processing that was still occurring. > > > > > > > >> AFAIR previous > >> thread, something else was suggested, it was about storing the fact > >> that user already received warning/info about error. > > > > Yes, that was the discussion. > > > > I went with this patch because: > > > > 1. It deals with the root cause: unneeded DRM KMS connector polling > > > > 2. It eliminates all my DRM KMS connector polling related log spam, not > > just the hex dump. > > > > 3. The existing sysfs structure was already exposing 'struct > > drm_connector' members to user-space at per connector granularity > > > > > > The real root cause of the polling spew is that the radeon module, based > > on the best information it has (BIOS connector tables, I2C bus probes, > > etc.), makes a decision on if a connector should be polled and how it > > should be polled. > > > > Given incorrect BIOS tables, and strange I2C setups, the radeon module > > is bound to make the wrong decision about polling in some cases. This > > change is a way for the human to step in a correct things, when the > > radeon driver gets it wrong. > > > > I'd rather fix the bug for real rather than providing users with an > out so that the bug never gets reported. There is already "an out" on it's way upstream: http://lkml.org/lkml/2010/9/6/375 http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commit;h=e58f637bb96d5a0ae0919b9998b891d1ba7e47c9 I noticed it after I submitted this patch. If I'm reading it right, the module parameter fix is very coarse: it applies to all drm adapters and connectors in the system. > What's the actual problem > you have? The actual problem I have is that the radeon driver reports three connectors (HDMI, DVI-D, and VGA) on the motherboard, but there is only one physical connector (VGA). Polling of the HDMI appears to be inhibited by the radeon module code. Polling of the (non-existent?) DVI-D interface yields a lot of log messages and useless I2C transactions. My precise gripe is with this code path http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob;f=drivers/gpu/drm/radeon/radeon_connectors.c;h=ecc1a8fafbfd3eb3c12c0c4d45b4b091a1bee03b;hb=refs/heads/drm-fixes#l772 which is being polled every 10 seconds. It is bad for me because this function is noisy: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob;f=drivers/gpu/drm/drm_edid.c;h=96e96310822513bfd9c984054a913eac7b5acc50;hb=refs/heads/drm-fixes#l136 and this (insane?) loop multiplies the noise by a factor of 4: http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob;f=drivers/gpu/drm/drm_edid.c;h=96e96310822513bfd9c984054a913eac7b5acc50;hb=refs/heads/drm-fixes#l264 In a different user's operational context, all that noise may be beneficial. The real problem to me is that the radeon and drm modules have a single, standard way of dealing with EDID errors. However, EDID errors can happen due to a number of different causes, some of which are external (i.e. in the LCD or CRT monitor). Given that, there really is no "right thing" the drivers can do without input from the user on what the policy should be when a bad EDID is detected. > Do yo
[PATCH v2 0/2] drm/sysfs: Provide per connector control of DRM KMS polling
Per Greg's request, here is a resend of my previous patch to add sysfs entries to allow manual override of DRM KMS connector polling actions, with accompanying documentation. Only documentation has been added. No code has changed from the previous patch submission. diffstat: Documentation/ABI/testing/sysfs-drm | 208 drivers/gpu/drm/drm_sysfs.c | 69 +++ 2 files changed, 277 insertions(+) Regards, Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/2] drm/sysfs: Provide per connector control of DRM KMS polling
DRM KMS polling of connections providing errant EDID responses, or
polling of connectors that have chips responding on DDC I2C bus
address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM,
will create perpetual noise in dmesg and the system log every 10
seconds. Currently the user has apparently little recourse to silence
these messages aside from replacing the offending cable, monitor, or
graphics adapter. That recourse is impossible for an unused DVI-D
connector of an internal graphics processor on a motherboard that
provides no physical DVI-D connector.
This change allows the root user to disable (and re-enable) DRM KMS
connector polling on a per connector basis via sysfs, like so:
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] connect disconnect
# echo /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
# echo connect hotplug_detectable \
/sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] [connect] disconnect
# echo /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
with the enabled poll types for the connector denoted in brackets: [].
This allows the root user to silence DRM KMS log spam for locally known
uncorrectable conditions.
Signed-off-by Andy Walls awa...@md.metrocast.net
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 86118a7..8e0807d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -318,11 +318,80 @@ static ssize_t select_subconnector_show(struct device
*device,
drm_get_dvi_i_select_name((int)subconnector));
}
+static const struct {
+ uint8_t mask;
+ const char *name;
+} polled_bit_names[] = {
+ { DRM_CONNECTOR_POLL_HPD,hotplug_detectable },
+ { DRM_CONNECTOR_POLL_CONNECT,connect},
+ { DRM_CONNECTOR_POLL_DISCONNECT, disconnect },
+};
+
+/*
+ * Return the decoded contents of connector-polled, using the names of the
+ * all the bit masks. Bits that are set, have their names enclosed in
brackets.
+ */
+static ssize_t polled_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ char *tmp;
+ int i;
+
+ tmp = buf;
+ for (i = 0; i ARRAY_SIZE(polled_bit_names); i++) {
+ if (connector-polled polled_bit_names[i].mask)
+ tmp += sprintf(tmp, [%s] , polled_bit_names[i].name);
+ else
+ tmp += sprintf(tmp, %s , polled_bit_names[i].name);
+ }
+
+ if (tmp != buf)
+ *(tmp - 1) = '\n';
+ return tmp - buf;
+}
+
+/*
+ * Change the state of connector-polled, given input bit-mask name-strings
+ * that are separated by space or newline.
+ */
+static ssize_t polled_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ const char *tmp;
+ int i;
+ uint8_t polled;
+
+ /* Polling of this connector will cease, if no match is made below */
+ polled = 0;
+
+ /* Incrementally split and parse the input */
+ while ((tmp = strsep((char **) buf, \n)) != NULL) {
+
+ /* Don't waste effort on multiple adjacent separators */
+ if (*tmp == '\0')
+ continue;
+
+ /* Check for a match with a connector poll type name */
+ for (i = 0; i ARRAY_SIZE(polled_bit_names); i++) {
+ if (!strncasecmp(tmp, polled_bit_names[i].name,
+strlen(polled_bit_names[i].name))) {
+ polled |= polled_bit_names[i].mask;
+ break;
+ }
+ }
+ }
+ connector-polled = polled;
+ return count;
+}
+
static struct device_attribute connector_attrs[] = {
__ATTR_RO(status),
__ATTR_RO(enabled),
__ATTR_RO(dpms),
__ATTR_RO(modes),
+ __ATTR(polled, 0644, polled_show, polled_store),
};
/* These attributes are for both DVI-I connectors and all types of tv-out. */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/2] Doc/ABI: sysfs-drm initial document; polled entry
This is the initial addition of documentation for the drm module's sysfs entries. It provides a drm sysfs entries overview, and a detailed description of the new drm per output connector polled entry in sysfs. Signed-of-by: Andy Walls awa...@md.metrocast.net diff --git a/Documentation/ABI/testing/sysfs-drm b/Documentation/ABI/testing/sysfs-drm new file mode 100644 index 000..18a017a --- /dev/null +++ b/Documentation/ABI/testing/sysfs-drm @@ -0,0 +1,208 @@ +Direct Rendering Infrastructure (DRI) Direct Rendering Manager (drm) module +and Translation Table Manager (ttm) module sysfs entries + +Example class view showing a single graphics adapter with three output +connectors, a special control device node for the graphics adapter, and the +Translation Table Manager (ttm) graphics memory manager virtual device: + +/sys/class/drm/ +├── card0 - ../../devices/.../drm/card0 +├── card0-DVI-D-1 - ../../devices/.../drm/card0/card0-DVI-D-1 +├── card0-HDMI Type A-1 - ../../devices/.../drm/card0/card0-HDMI Type A-1 +├── card0-VGA-1 - ../../devices/.../drm/card0/card0-VGA-1 +├── controlD64 - ../../devices/.../drm/controlD64 +├── ttm - ../../devices/virtual/drm/ttm +└── version + +6 directories, 1 file + + +Corresponding DRI device nodes for the example graphics adapter, with +additional ACL properties on card0 granting rw perms to the X/console user: + +crw-rw+ 1 root video 226, 0 2010-09-21 20:52 /dev/dri/card0 +crw-rw-rw-. 1 root video 226, 64 2010-09-21 20:51 /dev/dri/controlD64 + + +Example physical device view for a single graphics adapter with three output +connectors and a special control device node for the graphics adapter: + +/sys/devices/.../drm +├── card0 +│ ├── card0-DVI-D-1 +│ │ ├── device - ../../card0 +│ │ ├── dpms +│ │ ├── edid +│ │ ├── enabled +│ │ ├── modes +│ │ ├── polled +│ │ ├── power +│ │ │ ├── control +│ │ │ ├── runtime_active_time +│ │ │ ├── runtime_status +│ │ │ ├── runtime_suspended_time +│ │ │ ├── wakeup +│ │ │ └── wakeup_count +│ │ ├── status +│ │ ├── subsystem - ../../../../../../../class/drm +│ │ └── uevent +│ ├── card0-HDMI Type A-1 +│ │ ├── device - ../../card0 +│ │ ├── dpms +│ │ ├── edid +│ │ ├── enabled +│ │ ├── modes +│ │ ├── polled +│ │ ├── power +│ │ │ ├── control +│ │ │ ├── runtime_active_time +│ │ │ ├── runtime_status +│ │ │ ├── runtime_suspended_time +│ │ │ ├── wakeup +│ │ │ └── wakeup_count +│ │ ├── status +│ │ ├── subsystem - ../../../../../../../class/drm +│ │ └── uevent +│ ├── card0-VGA-1 +│ │ ├── device - ../../card0 +│ │ ├── dpms +│ │ ├── edid +│ │ ├── enabled +│ │ ├── modes +│ │ ├── polled +│ │ ├── power +│ │ │ ├── control +│ │ │ ├── runtime_active_time +│ │ │ ├── runtime_status +│ │ │ ├── runtime_suspended_time +│ │ │ ├── wakeup +│ │ │ └── wakeup_count +│ │ ├── status +│ │ ├── subsystem - ../../../../../../../class/drm +│ │ └── uevent +│ ├── dev +│ ├── device - ../../../:01:05.0 +│ ├── power +│ │ ├── control +│ │ ├── runtime_active_time +│ │ ├── runtime_status +│ │ ├── runtime_suspended_time +│ │ ├── wakeup +│ │ └── wakeup_count +│ ├── subsystem - ../../../../../../class/drm +│ └── uevent +└── controlD64 +├── dev +├── device - ../../../:01:05.0 +├── power +│ ├── control +│ ├── runtime_active_time +│ ├── runtime_status +│ ├── runtime_suspended_time +│ ├── wakeup +│ └── wakeup_count +├── subsystem - ../../../../../../class/drm +└── uevent + +20 directories, 55 files + + +Example virtual device view view showing the Translation Table Manager (ttm) +graphics memory manager virtual device: + +/sys/devices/virtual/drm +└── ttm +├── buffer_objects +│ └── bo_count +├── memory_accounting +│ ├── kernel +│ │ ├── available_memory +│ │ ├── emergency_memory +│ │ ├── swap_limit +│ │ ├── used_memory +│ │ └── zone_memory +│ └── pool +│ ├── pool_allocation_size +│ ├── pool_max_size +│ └── pool_small_allocation +├── power +│ ├── control +│ ├── runtime_active_time +│ ├── runtime_status +│ ├── runtime_suspended_time +│ ├── wakeup +│ └── wakeup_count +├── subsystem - ../../../../class/drm +└── uevent + +7 directories
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Mon, 2010-09-20 at 20:29 +0200, Rafa? Mi?ecki wrote:
> 2010/9/20 Andy Walls :
> > DRM KMS polling of connections providing errant EDID responses, or
> > polling of "connectors" that have chips responding on DDC I2C bus
> > address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM,
> > will create perpetual noise in dmesg and the system log every 10
> > seconds. Currently the user has apparently little recourse to silence
> > these messages aside from replacing the offending cable, monitor, or
> > graphics adapter. That recourse is impossible for an unused DVI-D
> > "connector" of an internal graphics processor on a motherboard that
> > provides no physical DVI-D connector.
> >
> > This change allows the root user to disable (and re-enable) DRM KMS
> > connector polling on a per connector basis via sysfs, like so:
>
> Huh, I didn't imagine solution of this issue that way.
The problems with my initial patch, and related approaches, were:
1. They didn't get rid of all the spam in the logs, just the most
verbose portion of it.
2. They didn't deal with the root cause of the log spam and all the
related, but unneeded, I/O and processing that was still occurring.
> AFAIR previous
> thread, something else was suggested, it was about storing the fact
> that user already received warning/info about error.
Yes, that was the discussion.
I went with this patch because:
1. It deals with the root cause: unneeded DRM KMS connector polling
2. It eliminates all my DRM KMS connector polling related log spam, not
just the hex dump.
3. The existing sysfs structure was already exposing 'struct
drm_connector' members to user-space at per connector granularity
The real root cause of the polling spew is that the radeon module, based
on the best information it has (BIOS connector tables, I2C bus probes,
etc.), makes a decision on if a connector should be polled and how it
should be polled.
Given incorrect BIOS tables, and strange I2C setups, the radeon module
is bound to make the wrong decision about polling in some cases. This
change is a way for the human to step in a correct things, when the
radeon driver gets it wrong.
In my case, along with the log spam, there are also a lot of unneeded
I2C transactions going on.
BTW, I found that Chris Wilson recently committed a change to inhibit
all drm connector polling globally for a different reason:
http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=commit;h=e58f637bb96d5a0ae0919b9998b891d1ba7e47c9
That commit message shows a case where the driver decides polling needs
to happen, but the human knows differently and manual control over
connector polling mitigates the problem.
> Why I don't really like proposed solution?
> 1) You need to be root
Yes, for both sysfs and module parameters.
I view video hardware configuration (montiors, cables, graphics cards)
and control of what gets logged into files as system administrative
issues. I'd assume that needing root privileges to turn on and off
device auto detection would be the norm. I imagined that root would set
up any DRM KMS connector polling fixes once in an init script and be
done with it. Is there any other use-case you had in mind?
If there's some set-uid or group privilege method or utilities that's in
common use (xrandr?) for manual control of drm/kms parameters, that you
think would be better, just point me in that direction and I'll try to
work something out.
However, trying to code automatic control of when to disable DRM KMS
connector polling, by some heuristic or algorithm in radeon or drm, is
likely doomed to failure for corner cases or for someone's policy
preferences. I'd rather not attempt that.
> 2) Need to know and play with some "magic" sysfs file.
I strongly dislike ever using sysfs myself. However, I'll spare us all
my rantings^Wreasons. ;)
My problem with KMS was with the documentation I could find. The
"userland interfaces" section in Documentation/Docbook/drm.tmpl is
essentially empty. DRM design documents at freedesktop.org (which
actually look closer to requirements documents) are the most detailed
documents I could find, but they don't really cover KMS. Maybe I'm just
looking in the wrong places.
Is there a KMS or DRM_MODESET API document? Maybe I could propose a new
ioctl(), if there isn't already one that will be usable for controlling
DRM KMS polling.
> I thing it would be much better and user-friendly to display such an
> error just once per display. How this could be implemented?
>
> 1) Add field like "warned" to connector struct, default to 0
> 2) if (warning_to_be_printed && !connector->warned) { printk();
> connector->warned = 1
> 3) if (connector_disconnected) { connector->warned = 0; }
>
> This was you will get just one warning
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Mon, 2010-09-20 at 11:52 -0700, Greg KH wrote: > On Mon, Sep 20, 2010 at 08:59:00AM -0400, Andy Walls wrote: > > This change allows the root user to disable (and re-enable) DRM KMS > > connector polling on a per connector basis via sysfs, like so: > > > > # cat /sys/class/drm/card0/card0-DVI-D-1/polled > > [hotplug_detectable] connect disconnect > You are adding a sysfs file, yet you forgot to add a file in > Documentation/ABI. Please fix that and resend the patch. Oops, process failure, sorry. Will do. Regards, Andy
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
DRM KMS polling of connections providing errant EDID responses, or
polling of "connectors" that have chips responding on DDC I2C bus
address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM,
will create perpetual noise in dmesg and the system log every 10
seconds. Currently the user has apparently little recourse to silence
these messages aside from replacing the offending cable, monitor, or
graphics adapter. That recourse is impossible for an unused DVI-D
"connector" of an internal graphics processor on a motherboard that
provides no physical DVI-D connector.
This change allows the root user to disable (and re-enable) DRM KMS
connector polling on a per connector basis via sysfs, like so:
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] connect disconnect
# echo > /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
# echo " connect hotplug_detectable " > \
/sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] [connect] disconnect
# echo > /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
with the enabled poll types for the connector denoted in brackets: [].
This allows the root user to silence DRM KMS log spam for locally known
uncorrectable conditions.
Signed-off-by Andy Walls
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 86118a7..8e0807d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -318,11 +318,80 @@ static ssize_t select_subconnector_show(struct device
*device,
drm_get_dvi_i_select_name((int)subconnector));
}
+static const struct {
+ uint8_t mask;
+ const char *name;
+} polled_bit_names[] = {
+ { DRM_CONNECTOR_POLL_HPD,"hotplug_detectable" },
+ { DRM_CONNECTOR_POLL_CONNECT,"connect"},
+ { DRM_CONNECTOR_POLL_DISCONNECT, "disconnect" },
+};
+
+/*
+ * Return the decoded contents of connector->polled, using the names of the
+ * all the bit masks. Bits that are set, have their names enclosed in
brackets.
+ */
+static ssize_t polled_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ char *tmp;
+ int i;
+
+ tmp = buf;
+ for (i = 0; i < ARRAY_SIZE(polled_bit_names); i++) {
+ if (connector->polled & polled_bit_names[i].mask)
+ tmp += sprintf(tmp, "[%s] ", polled_bit_names[i].name);
+ else
+ tmp += sprintf(tmp, "%s ", polled_bit_names[i].name);
+ }
+
+ if (tmp != buf)
+ *(tmp - 1) = '\n';
+ return tmp - buf;
+}
+
+/*
+ * Change the state of connector->polled, given input bit-mask name-strings
+ * that are separated by space or newline.
+ */
+static ssize_t polled_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ const char *tmp;
+ int i;
+ uint8_t polled;
+
+ /* Polling of this connector will cease, if no match is made below */
+ polled = 0;
+
+ /* Incrementally split and parse the input */
+ while ((tmp = strsep((char **) , " \n")) != NULL) {
+
+ /* Don't waste effort on multiple adjacent separators */
+ if (*tmp == '\0')
+ continue;
+
+ /* Check for a match with a connector poll type name */
+ for (i = 0; i < ARRAY_SIZE(polled_bit_names); i++) {
+ if (!strncasecmp(tmp, polled_bit_names[i].name,
+strlen(polled_bit_names[i].name))) {
+ polled |= polled_bit_names[i].mask;
+ break;
+ }
+ }
+ }
+ connector->polled = polled;
+ return count;
+}
+
static struct device_attribute connector_attrs[] = {
__ATTR_RO(status),
__ATTR_RO(enabled),
__ATTR_RO(dpms),
__ATTR_RO(modes),
+ __ATTR(polled, 0644, polled_show, polled_store),
};
/* These attributes are for both DVI-I connectors and all types of tv-out. */
Re: [PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On Sun, 2010-09-19 at 20:39 +0200, Rafał Miłecki wrote:
2010/9/18 Marcin Slusarz marcin.slus...@gmail.com:
On Fri, Sep 17, 2010 at 06:53:26PM -0400, Andy Walls wrote:
On my system, every 10 seconds drm_edid_block_valid() gets called 4
times by radeon_dvi_detect(). This results in 4 instances of a
multi-line hex dump of the same EDID (non-)data being logged every 10
seconds.
Silence the hex dump from drm_edid_block_valid() unless a drm_debug
module parameter flag is set.
Signed-of-by: Andy Walls awa...@md.metrocast.net
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dce5c4a..33a748c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
bad:
if (raw_edid) {
- DRM_ERROR(Raw EDID:\n);
- print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
EDID_LENGTH);
- printk(\n);
+ DRM_DEBUG(Raw EDID:\n);
+ if (drm_debug DRM_UT_CORE) {
+ print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
+raw_edid, EDID_LENGTH);
+ printk(\n);
+ }
}
return 0;
}
Why not print it only once on original error level?
Something like:
static bool printed = false;
if (!printed) {
...
printed = true;
}
It has the same effect for you (no spamming by default) and it's still
provide some information.
Should be per-monitor or per-output I think.
Yes, something like that. I thought about that after my reply.
My perpetual log spam is related to my DVI-D-1 connector for my radeon
graphics chip. So per connector, per graphics chip.
With the patch as provided, the hex dump messages can be turned on and
off using something like:
echo 1 /sys/module/drm/parameters/debug
echo 0 /sys/module/drm/parameters/debug
once the user has a shell prompt.
Regards,
Andy
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
DRM KMS polling of connections providing errant EDID responses, or
polling of connectors that have chips responding on DDC I2C bus
address 0xA0/0xA1 with no actual physical connector nor EDID EEPROM,
will create perpetual noise in dmesg and the system log every 10
seconds. Currently the user has apparently little recourse to silence
these messages aside from replacing the offending cable, monitor, or
graphics adapter. That recourse is impossible for an unused DVI-D
connector of an internal graphics processor on a motherboard that
provides no physical DVI-D connector.
This change allows the root user to disable (and re-enable) DRM KMS
connector polling on a per connector basis via sysfs, like so:
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] connect disconnect
# echo /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
# echo connect hotplug_detectable \
/sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
[hotplug_detectable] [connect] disconnect
# echo /sys/class/drm/card0/card0-DVI-D-1/polled
# cat /sys/class/drm/card0/card0-DVI-D-1/polled
hotplug_detectable connect disconnect
with the enabled poll types for the connector denoted in brackets: [].
This allows the root user to silence DRM KMS log spam for locally known
uncorrectable conditions.
Signed-off-by Andy Walls awa...@md.metrocast.net
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 86118a7..8e0807d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -318,11 +318,80 @@ static ssize_t select_subconnector_show(struct device
*device,
drm_get_dvi_i_select_name((int)subconnector));
}
+static const struct {
+ uint8_t mask;
+ const char *name;
+} polled_bit_names[] = {
+ { DRM_CONNECTOR_POLL_HPD,hotplug_detectable },
+ { DRM_CONNECTOR_POLL_CONNECT,connect},
+ { DRM_CONNECTOR_POLL_DISCONNECT, disconnect },
+};
+
+/*
+ * Return the decoded contents of connector-polled, using the names of the
+ * all the bit masks. Bits that are set, have their names enclosed in
brackets.
+ */
+static ssize_t polled_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ char *tmp;
+ int i;
+
+ tmp = buf;
+ for (i = 0; i ARRAY_SIZE(polled_bit_names); i++) {
+ if (connector-polled polled_bit_names[i].mask)
+ tmp += sprintf(tmp, [%s] , polled_bit_names[i].name);
+ else
+ tmp += sprintf(tmp, %s , polled_bit_names[i].name);
+ }
+
+ if (tmp != buf)
+ *(tmp - 1) = '\n';
+ return tmp - buf;
+}
+
+/*
+ * Change the state of connector-polled, given input bit-mask name-strings
+ * that are separated by space or newline.
+ */
+static ssize_t polled_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct drm_connector *connector = to_drm_connector(dev);
+ const char *tmp;
+ int i;
+ uint8_t polled;
+
+ /* Polling of this connector will cease, if no match is made below */
+ polled = 0;
+
+ /* Incrementally split and parse the input */
+ while ((tmp = strsep((char **) buf, \n)) != NULL) {
+
+ /* Don't waste effort on multiple adjacent separators */
+ if (*tmp == '\0')
+ continue;
+
+ /* Check for a match with a connector poll type name */
+ for (i = 0; i ARRAY_SIZE(polled_bit_names); i++) {
+ if (!strncasecmp(tmp, polled_bit_names[i].name,
+strlen(polled_bit_names[i].name))) {
+ polled |= polled_bit_names[i].mask;
+ break;
+ }
+ }
+ }
+ connector-polled = polled;
+ return count;
+}
+
static struct device_attribute connector_attrs[] = {
__ATTR_RO(status),
__ATTR_RO(enabled),
__ATTR_RO(dpms),
__ATTR_RO(modes),
+ __ATTR(polled, 0644, polled_show, polled_store),
};
/* These attributes are for both DVI-I connectors and all types of tv-out. */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/sysfs: Provide per connector control of DRM KMS polling
On Mon, 2010-09-20 at 11:52 -0700, Greg KH wrote: On Mon, Sep 20, 2010 at 08:59:00AM -0400, Andy Walls wrote: This change allows the root user to disable (and re-enable) DRM KMS connector polling on a per connector basis via sysfs, like so: # cat /sys/class/drm/card0/card0-DVI-D-1/polled [hotplug_detectable] connect disconnect You are adding a sysfs file, yet you forgot to add a file in Documentation/ABI. Please fix that and resend the patch. Oops, process failure, sorry. Will do. Regards, Andy ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On Sun, 2010-09-19 at 20:39 +0200, Rafa? Mi?ecki wrote:
> 2010/9/18 Marcin Slusarz :
> > On Fri, Sep 17, 2010 at 06:53:26PM -0400, Andy Walls wrote:
> >> On my system, every 10 seconds drm_edid_block_valid() gets called 4
> >> times by radeon_dvi_detect(). This results in 4 instances of a
> >> multi-line hex dump of the same EDID (non-)data being logged every 10
> >> seconds.
> >>
> >> Silence the hex dump from drm_edid_block_valid() unless a drm_debug
> >> module parameter flag is set.
> >>
> >> Signed-of-by: Andy Walls
> >>
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index dce5c4a..33a748c 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
> >>
> >> bad:
> >> if (raw_edid) {
> >> - DRM_ERROR("Raw EDID:\n");
> >> - print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
> >> EDID_LENGTH);
> >> - printk("\n");
> >> + DRM_DEBUG("Raw EDID:\n");
> >> + if (drm_debug & DRM_UT_CORE) {
> >> + print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
> >> +raw_edid, EDID_LENGTH);
> >> + printk("\n");
> >> + }
> >> }
> >> return 0;
> >> }
> >>
> >
> > Why not print it only once on original error level?
> > Something like:
> > static bool printed = false;
> > if (!printed) {
> >...
> >printed = true;
> > }
> >
> > It has the same effect for you (no spamming by default) and it's still
> > provide some information.
>
> Should be per-monitor or per-output I think.
Yes, something like that. I thought about that after my reply.
My perpetual log spam is related to my DVI-D-1 "connector" for my radeon
graphics chip. So per connector, per graphics chip.
With the patch as provided, the hex dump messages can be turned on and
off using something like:
echo 1 > /sys/module/drm/parameters/debug
echo 0 > /sys/module/drm/parameters/debug
once the user has a shell prompt.
Regards,
Andy
Re: [PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On Sat, 2010-09-18 at 13:50 +0200, Marcin Slusarz wrote:
On Fri, Sep 17, 2010 at 06:53:26PM -0400, Andy Walls wrote:
On my system, every 10 seconds drm_edid_block_valid() gets called 4
times by radeon_dvi_detect(). This results in 4 instances of a
multi-line hex dump of the same EDID (non-)data being logged every 10
seconds.
Silence the hex dump from drm_edid_block_valid() unless a drm_debug
module parameter flag is set.
Signed-of-by: Andy Walls awa...@md.metrocast.net
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dce5c4a..33a748c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
bad:
if (raw_edid) {
- DRM_ERROR(Raw EDID:\n);
- print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
EDID_LENGTH);
- printk(\n);
+ DRM_DEBUG(Raw EDID:\n);
+ if (drm_debug DRM_UT_CORE) {
+ print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
+raw_edid, EDID_LENGTH);
+ printk(\n);
+ }
}
return 0;
}
Why not print it only once on original error level?
Something like:
static bool printed = false;
if (!printed) {
...
printed = true;
}
It has the same effect for you (no spamming by default) and it's still
provide some information.
That's acceptable to me. I'm assuming that if the radeon.ko and drm.ko
modules get loaded early (from initrd), that the log instance will still
be in the dmesg ring buffer for when the OS can finally log dmesg to
disk.
I'm still getting spammed every 10 seconds. It's just not as bad now:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: radeon :01:05.0: DVI-D-1: EDID
invalid.
Sep 17 11:56:26 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1:
probed a monitor but no|invalid EDID
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: radeon :01:05.0: DVI-D-1: EDID
invalid.
Sep 17 11:56:26 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1:
probed a monitor but no|invalid EDID
My MSI-7302 motherboard apparently has a graphics chipset that supports
DVI, but the motherboard has no physical DVI connector - only a VGA
connector. The invalid EDID data is just junk, and not real data from
that I2C(?) bus. lspci output on the PCIe graphics chipset is below.
If there's any easy way to just stop all the spam about the DVI
connector, I'd be interested.
Regards,
Andy
# lspci -tvnn
-[:00]-+-00.0 ATI Technologies Inc RS690 Host Bridge [1002:7911]
+-01.0-[01]05.0 ATI Technologies Inc Radeon 2100 [1002:796e]
+-04.0-[02]00.0 Conexant Systems, Inc. Device [14f1:8880]
+-05.0-[03]00.0 Realtek Semiconductor Co., Ltd. RTL8111/8168B
PCI Express Gigabit Ethernet controller [10ec:8168]
+-11.0 ATI Technologies Inc SB700/SB800 SATA Controller [IDE mode]
[1002:4390]
+-12.0 ATI Technologies Inc SB700/SB800 USB OHCI0 Controller
[1002:4397]
+-12.1 ATI Technologies Inc SB700 USB OHCI1 Controller [1002:4398]
+-12.2 ATI Technologies Inc SB700/SB800 USB EHCI Controller
[1002:4396]
+-13.0 ATI Technologies Inc SB700/SB800 USB OHCI0 Controller
[1002:4397]
+-13.1 ATI Technologies Inc SB700 USB OHCI1 Controller [1002:4398]
+-13.2 ATI Technologies Inc SB700/SB800 USB EHCI Controller
[1002:4396]
+-14.0 ATI Technologies Inc SBx00 SMBus Controller [1002:4385]
+-14.1 ATI Technologies Inc SB700/SB800 IDE Controller [1002:439c]
+-14.2 ATI Technologies Inc SBx00 Azalia (Intel HDA) [1002:4383]
+-14.3 ATI Technologies Inc SB700/SB800 LPC host controller
[1002:439d]
+-14.4-[04]02.0 Internext Compression Inc iTVC15 MPEG-2 Encoder
[:0803]
+-14.5 ATI Technologies Inc SB700/SB800
[PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On Sat, 2010-09-18 at 13:50 +0200, Marcin Slusarz wrote:
> On Fri, Sep 17, 2010 at 06:53:26PM -0400, Andy Walls wrote:
> > On my system, every 10 seconds drm_edid_block_valid() gets called 4
> > times by radeon_dvi_detect(). This results in 4 instances of a
> > multi-line hex dump of the same EDID (non-)data being logged every 10
> > seconds.
> >
> > Silence the hex dump from drm_edid_block_valid() unless a drm_debug
> > module parameter flag is set.
> >
> > Signed-of-by: Andy Walls
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index dce5c4a..33a748c 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
> >
> > bad:
> > if (raw_edid) {
> > - DRM_ERROR("Raw EDID:\n");
> > - print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
> > EDID_LENGTH);
> > - printk("\n");
> > + DRM_DEBUG("Raw EDID:\n");
> > + if (drm_debug & DRM_UT_CORE) {
> > + print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
> > +raw_edid, EDID_LENGTH);
> > + printk("\n");
> > + }
> > }
> > return 0;
> > }
> >
>
> Why not print it only once on original error level?
> Something like:
> static bool printed = false;
> if (!printed) {
> ...
> printed = true;
> }
>
> It has the same effect for you (no spamming by default) and it's still
> provide some information.
That's acceptable to me. I'm assuming that if the radeon.ko and drm.ko
modules get loaded early (from initrd), that the log instance will still
be in the dmesg ring buffer for when the OS can finally log dmesg to
disk.
I'm still getting spammed every 10 seconds. It's just not as bad now:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: radeon :01:05.0: DVI-D-1: EDID
invalid.
Sep 17 11:56:26 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1:
probed a monitor but no|invalid EDID
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: [drm:drm_edid_is_valid] *ERROR* Raw EDID:
Sep 17 11:56:26 morgan kernel:
Sep 17 11:56:26 morgan kernel: radeon :01:05.0: DVI-D-1: EDID
invalid.
Sep 17 11:56:26 morgan kernel: [drm:radeon_dvi_detect] *ERROR* DVI-D-1:
probed a monitor but no|invalid EDID
My MSI-7302 motherboard apparently has a graphics chipset that supports
DVI, but the motherboard has no physical DVI connector - only a VGA
connector. The invalid EDID data is just junk, and not real data from
that I2C(?) bus. lspci output on the PCIe graphics chipset is below.
If there's any easy way to just stop all the spam about the DVI
connector, I'd be interested.
Regards,
Andy
# lspci -tvnn
-[:00]-+-00.0 ATI Technologies Inc RS690 Host Bridge [1002:7911]
+-01.0-[01]05.0 ATI Technologies Inc Radeon 2100 [1002:796e]
+-04.0-[02]00.0 Conexant Systems, Inc. Device [14f1:8880]
+-05.0-[03]00.0 Realtek Semiconductor Co., Ltd. RTL8111/8168B
PCI Express Gigabit Ethernet controller [10ec:8168]
+-11.0 ATI Technologies Inc SB700/SB800 SATA Controller [IDE mode]
[1002:4390]
+-12.0 ATI Technologies Inc SB700/SB800 USB OHCI0 Controller
[1002:4397]
+-12.1 ATI Technologies Inc SB700 USB OHCI1 Controller [1002:4398]
+-12.2 ATI Technologies Inc SB700/SB800 USB EHCI Controller
[1002:4396]
+-13.0 ATI Technologies Inc SB700/SB800 USB OHCI0 Controller
[1002:4397]
+-13.1 ATI Technologies Inc SB700 USB OHCI1 Controller [1002:4398]
+-13.2 ATI Technologies Inc SB700/SB800 USB EHCI Controller
[1002:4396]
+-14.0 ATI Technologies Inc SBx00 SMBus Controller [1002:4385]
+-14.1 ATI Technologies Inc SB700/SB
[PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On my system, every 10 seconds drm_edid_block_valid() gets called 4
times by radeon_dvi_detect(). This results in 4 instances of a
multi-line hex dump of the same EDID (non-)data being logged every 10
seconds.
Silence the hex dump from drm_edid_block_valid() unless a drm_debug
module parameter flag is set.
Signed-of-by: Andy Walls awa...@md.metrocast.net
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dce5c4a..33a748c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
bad:
if (raw_edid) {
- DRM_ERROR(Raw EDID:\n);
- print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
EDID_LENGTH);
- printk(\n);
+ DRM_DEBUG(Raw EDID:\n);
+ if (drm_debug DRM_UT_CORE) {
+ print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
+raw_edid, EDID_LENGTH);
+ printk(\n);
+ }
}
return 0;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/edid: Don't repeatedly log hex dumps of bad EDIDs by default
On my system, every 10 seconds drm_edid_block_valid() gets called 4
times by radeon_dvi_detect(). This results in 4 instances of a
multi-line hex dump of the same EDID (non-)data being logged every 10
seconds.
Silence the hex dump from drm_edid_block_valid() unless a drm_debug
module parameter flag is set.
Signed-of-by: Andy Walls
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index dce5c4a..33a748c 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -173,9 +173,12 @@ drm_edid_block_valid(u8 *raw_edid)
bad:
if (raw_edid) {
- DRM_ERROR("Raw EDID:\n");
- print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid,
EDID_LENGTH);
- printk("\n");
+ DRM_DEBUG("Raw EDID:\n");
+ if (drm_debug & DRM_UT_CORE) {
+ print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE,
+raw_edid, EDID_LENGTH);
+ printk("\n");
+ }
}
return 0;
}
