Re: [Linaro-mm-sig] [PATCH 1/4] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
On Fri, Aug 10, 2012 at 2:32 PM, Daniel Vetter wrote: > On Fri, Aug 10, 2012 at 04:57:43PM +0200, Maarten Lankhorst wrote: >> Documentation says that code requiring dma-buf should add it to >> select, so inline fallbacks are not going to be used. A link error >> will make it obvious what went wrong, instead of silently doing >> nothing at runtime. >> >> Signed-off-by: Maarten Lankhorst > > I've botched it more than once to update these when creating new dma-buf > code. Hence > > Reviewed-by: Daniel Vetter yeah, I think the fallbacks date back to when it was a user configurable option, rather than something select'd by drivers using dmabuf, and we just never went back to clean up. Let's drop the fallbacks. Reviewed-by: Rob Clark > -- > Daniel Vetter > Mail: dan...@ffwll.ch > Mobile: +41 (0)79 365 57 48 > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: EDID quirk improvements
On 08/11/2012 03:31 AM, Paul Menzel wrote: > As a side note, could you also mention the patch iteration in the tag, > that means [PATCH vN] so that I know what is the latest version. That > would be great. Can you (or anyone else reading this) point me to how to do this with git send-email? > I would submit adding the new quirk flags and the LG quirk as separate > patches. If it is not too much work, it would be great if you could > split them up. Does git provide any facility to make this easier? As far as I can tell, the process is to start over with a newly cloned repository, apply the current patch, manually back out the changes that I want to separate, commit, manually redo the separate changes, and commit again. Is that correct? > You even added this one from the following commit. > > commit bc42aabc6a01b92b0f961d65671564e0e1cd7592 > Author: Adam Jackson > Date: Wed May 23 16:26:54 2012 -0400 > > drm/edid/quirks: ViewSonic VA2026w > > I am going to need that quirk. Great! It's in Linus's tree. >> +DEFINE_MUTEX(edid_quirk_list_mutex); >> + >> +/** >> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for >> printing >> + * @mfg_id: the encoded manufacturer ID >> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 >> bytes) > > format*t*ed > > http://www.merriam-webster.com/dictionary/formatted Thanks for catching that. I keep finding little typos like that; very annoying. >> + >> +return count; >> +} > > Add an empty line here before the next comment? > >> /*** DDC fetch and block validation ***/ I'm pretty sure I can manage that. ;-) >> +MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt"); > > Not all users have access to the Linux source tree, so maybe a small > overview is still needed? Or even an URL? I can't think of a way to provide anything useful within the scope of a parameter description. Any suggestions? A URL would be great, but what would it be? (I don't have a personal web site, and that doesn't seem really appropriate anyway.) > Thanks again for that great patch. With the comments addressed above you > can add my acknowledgment. > > Acked-by: Paul Menzel Thank you for your feedback. Are you saying that I should add the acked-by? If so, how? (You can probably tell that I'm really struggling with git.) > I am going to try to test that patch too for a Philips and LG TV [2]. I hope it helps. -- Ian Pilcher arequip...@gmail.com "If you're going to shift my paradigm ... at least buy me dinner first." ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
Hey,
Op 11-08-12 17:14, Rob Clark schreef:
> On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
>> On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
>>> A dma-fence can be attached to a buffer which is being filled or consumed
>>> by hw, to allow userspace to pass the buffer without waiting to another
>>> device. For example, userspace can call page_flip ioctl to display the
>>> next frame of graphics after kicking the GPU but while the GPU is still
>>> rendering. The display device sharing the buffer with the GPU would
>>> attach a callback to get notified when the GPU's rendering-complete IRQ
>>> fires, to update the scan-out address of the display, without having to
>>> wake up userspace.
>>>
>>> A dma-fence is transient, one-shot deal. It is allocated and attached
>>> to one or more dma-buf's. When the one that attached it is done, with
>>> the pending operation, it can signal the fence.
>>>
>>> + dma_fence_signal()
>>>
>>> The dma-buf-mgr handles tracking, and waiting on, the fences associated
>>> with a dma-buf.
>>>
>>> TODO maybe need some helper fxn for simple devices, like a display-
>>> only drm/kms device which simply wants to wait for exclusive fence to
>>> be signaled, and then attach a non-exclusive fence while scanout is in
>>> progress.
>>>
>>> The one pending on the fence can add an async callback:
>>> + dma_fence_add_callback()
>>> The callback can optionally be cancelled with remove_wait_queue()
>>>
>>> Or wait synchronously (optionally with timeout or interruptible):
>>> + dma_fence_wait()
>>>
>>> A default software-only implementation is provided, which can be used
>>> by drivers attaching a fence to a buffer when they have no other means
>>> for hw sync. But a memory backed fence is also envisioned, because it
>>> is common that GPU's can write to, or poll on some memory location for
>>> synchronization. For example:
>>>
>>> fence = dma_buf_get_fence(dmabuf);
>>> if (fence->ops == &bikeshed_fence_ops) {
>>> dma_buf *fence_buf;
>>> dma_bikeshed_fence_get_buf(fence, &fence_buf, &offset);
>>> ... tell the hw the memory location to wait on ...
>>> } else {
>>> /* fall-back to sw sync * /
>>> dma_fence_add_callback(fence, my_cb);
>>> }
>>>
>>> On SoC platforms, if some other hw mechanism is provided for synchronizing
>>> between IP blocks, it could be supported as an alternate implementation
>>> with it's own fence ops in a similar way.
>>>
>>> To facilitate other non-sw implementations, the enable_signaling callback
>>> can be used to keep track if a device not supporting hw sync is waiting
>>> on the fence, and in this case should arrange to call dma_fence_signal()
>>> at some point after the condition has changed, to notify other devices
>>> waiting on the fence. If there are no sw waiters, this can be skipped to
>>> avoid waking the CPU unnecessarily. The handler of the enable_signaling
>>> op should take a refcount until the fence is signaled, then release its ref.
>>>
>>> The intention is to provide a userspace interface (presumably via eventfd)
>>> later, to be used in conjunction with dma-buf's mmap support for sw access
>>> to buffers (or for userspace apps that would prefer to do their own
>>> synchronization).
>> I think the commit message should be cleaned up: Kill the TODO, rip out
>> the bikeshed_fence and otherwise update it to the latest code.
Agreed.
>>> v1: Original
>>> v2: After discussion w/ danvet and mlankhorst on #dri-devel, we decided
>>> that dma-fence didn't need to care about the sw->hw signaling path
>>> (it can be handled same as sw->sw case), and therefore the fence->ops
>>> can be simplified and more handled in the core. So remove the signal,
>>> add_callback, cancel_callback, and wait ops, and replace with a simple
>>> enable_signaling() op which can be used to inform a fence supporting
>>> hw->hw signaling that one or more devices which do not support hw
>>> signaling are waiting (and therefore it should enable an irq or do
>>> whatever is necessary in order that the CPU is notified when the
>>> fence is passed).
>>> v3: Fix locking fail in attach_fence() and get_fence()
>>> v4: Remove tie-in w/ dma-buf.. after discussion w/ danvet and mlankorst
>>> we decided that we need to be able to attach one fence to N dma-buf's,
>>> so using the list_head in dma-fence struct would be problematic.
>>> v5: [ Maarten Lankhorst ] Updated for dma-bikeshed-fence and
>>> dma-buf-manager.
>>> v6: [ Maarten Lankhorst ] I removed dma_fence_cancel_callback and some
>>> comments
>>> about checking if fence fired or not. This is broken by design.
>>> waitqueue_active during destruction is now fatal, since the signaller
>>> should be holding a reference in enable_signalling until it signalled
>>> the fence. Pass the original dma_fence_cb along, and call __remove_wait
>>> in the dma_fence_callback handler, so that no cleanup needs to be
>>> perform
Re: [Linaro-mm-sig] [PATCH 3/4] dma-seqno-fence: Hardware dma-buf implementation of fencing (v2)
Op 10-08-12 21:57, Daniel Vetter schreef:
> On Fri, Aug 10, 2012 at 04:57:58PM +0200, Maarten Lankhorst wrote:
>> This type of fence can be used with hardware synchronization for simple
>> hardware that can block execution until the condition
>> (dma_buf[offset] - value) >= 0 has been met.
>>
>> A software fallback still has to be provided in case the fence is used
>> with a device that doesn't support this mechanism. It is useful to expose
>> this for graphics cards that have an op to support this.
>>
>> Some cards like i915 can export those, but don't have an option to wait,
>> so they need the software fallback.
>>
>> I extended the original patch by Rob Clark.
>>
>> v1: Original
>> v2: Renamed from bikeshed to seqno, moved into dma-fence.c since
>> not much was left of the file. Lots of documentation added.
>>
>> Signed-off-by: Maarten Lankhorst
> Patch looks good, two bikesheds inline. Either way
> Reviewed-by: Daniel Vetter
>
>> ---
>> drivers/base/dma-fence.c | 21 +++
>> include/linux/dma-fence.h | 61
>> +
>> 2 files changed, 82 insertions(+)
>>
>> diff --git a/drivers/base/dma-fence.c b/drivers/base/dma-fence.c
>> index 93448e4..4092a58 100644
>> --- a/drivers/base/dma-fence.c
>> +++ b/drivers/base/dma-fence.c
>> @@ -266,3 +266,24 @@ struct dma_fence *dma_fence_create(void *priv)
>> return fence;
>> }
>> EXPORT_SYMBOL_GPL(dma_fence_create);
>> +
>> +static int seqno_enable_signaling(struct dma_fence *fence)
>> +{
>> +struct dma_seqno_fence *seqno_fence = to_seqno_fence(fence);
>> +return seqno_fence->enable_signaling(seqno_fence);
>> +}
>> +
>> +static void seqno_release(struct dma_fence *fence)
>> +{
>> +struct dma_seqno_fence *f = to_seqno_fence(fence);
>> +
>> +if (f->release)
>> +f->release(f);
>> +dma_buf_put(f->sync_buf);
>> +}
>> +
>> +const struct dma_fence_ops dma_seqno_fence_ops = {
>> +.enable_signaling = seqno_enable_signaling,
>> +.release = seqno_release
>> +};
>> +EXPORT_SYMBOL_GPL(dma_seqno_fence_ops);
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index e0ceddd..3ef0da0 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -91,6 +91,19 @@ struct dma_fence_ops {
>> void (*release)(struct dma_fence *fence);
>> };
>>
>> +struct dma_seqno_fence {
>> +struct dma_fence base;
>> +
>> +struct dma_buf *sync_buf;
>> +uint32_t seqno_ofs;
>> +uint32_t seqno;
>> +
>> +int (*enable_signaling)(struct dma_seqno_fence *fence);
>> +void (*release)(struct dma_seqno_fence *fence);
> I think using dma_fence_ops here is the better color. We lose type-safety
> at compile-time, but still keep type-safety at runtime (thanks to
> to_dma_seqno_fence). In addition people seem to like to constify function
> pointers, we'd save a pointer and if we extend the sw dma_fence interface.
Ok, will change.
>> +};
>> +
>> +extern const struct dma_fence_ops dma_seqno_fence_ops;
>> +
>> struct dma_fence *dma_fence_create(void *priv);
>>
>> /**
>> @@ -121,4 +134,52 @@ int dma_fence_wait(struct dma_fence *fence, bool intr,
>> unsigned long timeout);
>> int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>> dma_fence_func_t func, void *priv);
>>
>> +/**
>> + * to_seqno_fence - cast a dma_fence to a dma_seqno_fence
>> + * @fence: dma_fence to cast to a dma_seqno_fence
>> + *
>> + * Returns NULL if the dma_fence is not a dma_seqno_fence,
>> + * or the dma_seqno_fence otherwise.
>> + */
>> +static inline struct dma_seqno_fence *
>> +to_seqno_fence(struct dma_fence *fence)
>> +{
>> +if (fence->ops != &dma_seqno_fence_ops)
>> +return NULL;
>> +return container_of(fence, struct dma_seqno_fence, base);
>> +}
> I think adding an is_dma_seqno_fence would be nice ...
#define is_dma_seqno_fence !!to_dma_seqno_fence
The first thing you would do after finding out it's a seqno fence
is calling to_dma_seqno_fence, otherwise why would you care?
As such the check was pointless and deleted.
My bikeshed, go build your own!
>> +
>> +/**
>> + * dma_seqno_fence_init - initialize a seqno fence
>> + * @fence: dma_seqno_fence to initialize
>> + * @sync_buf: buffer containing the memory location to signal on
>> + * @seqno_ofs: the offset within @sync_buf
>> + * @seqno: the sequence # to signal on
>> + * @priv: value of priv member
>> + * @enable_signaling: callback which is called when some other device is
>> + *waiting for sw notification of fence
>> + * @release: callback called during destruction before object is freed.
>> + *
>> + * This function initializes a struct dma_seqno_fence with passed
>> parameters,
>> + * and takes a reference on sync_buf which is released on fence destruction.
>> + */
>> +static inline void
>> +dma_seqno_fence_init(struct dma_seqno_fence *fence,
>> +struct dma_buf *sync_buf,
>> +uint32_t seqno
Re: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 10:14:40AM -0500, Rob Clark wrote:
> On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> > On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
> >> +
> >> + if (!ret) {
> >> + cb->base.flags = 0;
> >> + cb->base.func = __dma_fence_wake_func;
> >> + cb->base.private = priv;
> >> + cb->fence = fence;
> >> + cb->func = func;
> >> + __add_wait_queue(&fence->event_queue, &cb->base);
> >> + }
> >> + spin_unlock_irqrestore(&fence->event_queue.lock, flags);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dma_fence_add_callback);
> >
> > I think for api completenes we should also have a
> > dma_fence_remove_callback function.
>
> We did originally but Maarten found it was difficult to deal with
> properly when the gpu's hang. I think his alternative was just to
> require the hung driver to signal the fence. I had kicked around the
> idea of a dma_fence_cancel() alternative to signal that could pass an
> error thru to the waiting driver.. although not sure if the other
> driver could really do anything differently at that point.
Well, the idea is not to cancel all callbacks, but just a single one, in
case a driver wants to somehow abort the wait. E.g. when the own gpu dies
I guess we should clear all these fence callbacks so that they can't
clobber the hw state after the reset.
> >> +
> >> +/**
> >> + * dma_fence_wait - wait for a fence to be signaled
> >> + *
> >> + * @fence: [in]The fence to wait on
> >> + * @intr:[in]if true, do an interruptible wait
> >> + * @timeout: [in]absolute time for timeout, in jiffies.
> >
> > I don't quite like this, I think we should keep the styl of all other
> > wait_*_timeout functions and pass the arg as timeout in jiffies (and also
> > the same return semantics). Otherwise well have funny code that needs to
> > handle return values differently depending upon whether it waits upon a
> > dma_fence or a native object (where it would us the wait_*_timeout
> > functions directly).
>
> We did start out this way, but there was an ugly jiffies roll-over
> problem that was difficult to deal with properly. Using an absolute
> time avoided the problem.
Well, as-is the api works differently than all the other _timeout apis
I've seen in the kernel, which makes it confusing. Also, I don't quite see
what jiffies wraparound issue there is?
> > Also, I think we should add the non-_timeout variants, too, just for
> > completeness.
This request here has the same reasons, essentially: If we offer a
dma_fence wait api that matches the usual wait apis closely, it's harder
to get their usage wrong. I know that i915 has some major cludge for a
wait_seqno interface internally, but that's no reason to copy that
approach ;-)
Cheers, Daniel
--
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[ANNOUNCE] libdrm 2.4.38
Alex Deucher (2): radeon: add some missing evergreen pci ids radeon: add some new SI pci ids Chris Wilson (1): intel: Bail gracefully if we encounter an unknown Intel device Cooper Yuan (1): libdrm/exynos: padding gem_mmap structure to 64-bit aligned Damien Lespiau (1): intel: Remove two unused variables Dave Airlie (4): libdrm: add missing caps from kernel to drm.h libdrm: add prime fd->handle and handle->fd interfaces libdrm/nouveau: add prime handle->bo and bo->handle support. intel: add prime interface for getting/setting a prime bo. (v4) Eric Anholt (4): intel: Quiet valgrind warnings in context creation. Drop "-Wunsafe-loop-optimizations". intel: Import updated i915_drm.h. intel: Add a function for the new register read ioctl. Kenneth Graunke (1): intel: Change context create failure message to from fprintf to DBG(). Laurent Pinchart (8): modetest: Unify buffer allocation modetest: Add SMPTE test pattern modetest: Add test pattern support for missing packed YUV formats modetest: Add test pattern support for missing planar YUV formats modetest: Add test pattern support for missing RGB formats modetest: Move connector and plane parsing to separate functions modetest: Make frame buffer format configurable on the command line modeset: Split buffer allocation to a separate file Lauri Kasanen (1): intel: Fix build failure in test_decode.c Marek Olšák (6): radeon: simplify ZS buffer checking on r600 radeon: optimize allocation for depth w/o stencil and stencil w/o depth on EG radeon: force 2D tiling for MSAA surfaces radeon: tweak TILE_SPLIT for MSAA surfaces tests/modetest: fix distcheck configure: bump version for 2.4.38 release Paulo Zanoni (1): intel: add more Haswell PCI IDs Rob Clark (5): omap: clarify dmabuf file descriptor ownership omap: add API to import bo's from dmabuf fd's omap: add refcnting and handle tracking intel: fix build error modetest: fix uninitialized fourcc git tag: libdrm-2.4.38 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.38.tar.bz2 MD5: 8018e0bce5059cee05d855503d262cce libdrm-2.4.38.tar.bz2 SHA1: 21718ddb8be71bc74845a33c2b4fbda1de942e16 libdrm-2.4.38.tar.bz2 SHA256: a7caec55c945f7f8dec55fea9200391a168298bcdc4cb9a93c976e748193171a libdrm-2.4.38.tar.bz2 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.38.tar.gz MD5: 9aa553e543d716f2f9b9405274c5d9c3 libdrm-2.4.38.tar.gz SHA1: 8a98f0332b33d647918cc8c4bf41fbe1e21a15cb libdrm-2.4.38.tar.gz SHA256: 75d586a5a989a572e63a8f1252a51dfee4cdeb0146f8b9fe5ecd1b6844e85a20 libdrm-2.4.38.tar.gz ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: EDID quirk improvements
Dear Ian, Am Freitag, den 10.08.2012, 13:44 -0500 schrieb Ian Pilcher: […] > diff --git a/Documentation/EDID/edid_quirks.txt > b/Documentation/EDID/edid_quirks.txt > new file mode 100644 > index 000..256ded0 > --- /dev/null > +++ b/Documentation/EDID/edid_quirks.txt […] > +Overview > + > + > +EDID quirks provide a mechanism for working around display hardware with > buggy > +EDID data. > + > +An individual EDID quirk maps a display type (identified by its EDID > +manufacturer ID and product code[1]) to a set of flags. For example, the > current > +list of quirks built into the kernel is: > + > +ACR:0xad46:0x0001 > +API:0x7602:0x0001 > +ACR:0x0977:0x0020 > +MAX:0x05ec:0x0001 > +MAX:0x077e:0x0001 > +EPI:0xe780:0x0002 > +EPI:0x2028:0x0001 > +FCM:0x3520:0x000c > +LPL:0x:0x0010 > +LPL:0x2a00:0x0010 > +PHL:0xe014:0x0020 > +PTS:0x02fd:0x0020 > +SAM:0x021d:0x0040 > +SAM:0x0254:0x0001 > +SAM:0x027e:0x0001 > +VSC:0x139c:0x0080 > +GSM:0x563f:0x0300 reading the document again, I guess keeping this list up to date will be forgotten and duplicating this information is not necessary. Maybe just add one or two example quirks. I cannot think of a `grep` command to run to list all quirks, so maybe just mention that all the quirks are stored in `edid_quirk_list[]` in `drivers/gpu/drm/drm_edid.c` [1]. > + > +The first field of each quirk is the manufacturer ID, the second field is the > +product code, and the third field is the quirk flags. > + > +NOTE: All of the manufacturer IDs above are displayed as 3-character strings, > +because they are conformant IDs that have been properly encoded: > + > +- The most-significant bit of the encoded ID is 0 > +- They only contain ASCII characters in the range A-Z > + > +IDs that do not conform to these rules are displayed as "raw" hexadecimal > +values. > + > +The current quirk flags are: > + > +/* First detailed mode wrong, use largest 60Hz mode */ > +#define EDID_QUIRK_PREFER_LARGE_60 0x0001 > + > +/* Reported 135MHz pixel clock is too high, needs adjustment */ > +#define EDID_QUIRK_135_CLOCK_TOO_HIGH 0x0002 > + > +/* Prefer the largest mode at 75 Hz */ > +#define EDID_QUIRK_PREFER_LARGE_75 0x0004 > + > +/* Detail timing is in cm not mm */ > +#define EDID_QUIRK_DETAILED_IN_CM 0x0008 > + > +/* Detailed timing descriptors have bogus size values, so just take the > + * maximum size and use that. > + */ > +#define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE0x0010 > + > +/* Monitor forgot to set the first detailed is preferred bit. */ > +#define EDID_QUIRK_FIRST_DETAILED_PREFERRED 0x0020 > + > +/* use +hsync +vsync for detailed mode */ > +#define EDID_QUIRK_DETAILED_SYNC_PP 0x0040 > + > +/* Force reduced-blanking timings for detailed modes */ > +#define EDID_QUIRK_FORCE_REDUCED_BLANKING 0x0080 > + > +/* Display is confused by InfoFrames; don't sent any */ > +#define EDID_QUIRK_DISABLE_INFOFRAMES 0x0100 > + > +/* Display doesn't have any audio output */ > +#define EDID_QUIRK_NO_AUDIO 0x0200 That might be also hard to keep up to date. Maybe also just note that these quirks are defined in the beginning of `drivers/gpu/drm/drm_edid.c` [1] and that these are bit shifts(?) [2]. [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c;h=a8743c399e83234c976ebdb4b471542a0645c42d;hb=HEAD [2] https://en.wikipedia.org/wiki/Bitwise_operation#Shifts_in_C.2C_C.2B.2B.2C_C.23 […] Thanks, Paul signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 06:00:46PM +0200, Maarten Lankhorst wrote:
> Op 11-08-12 17:14, Rob Clark schreef:
> > On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> >>> +/**
> >>> + * dma_fence_signal - signal completion of a fence
> >>> + * @fence: the fence to signal
> >>> + *
> >>> + * All registered callbacks will be called directly (synchronously) and
> >>> + * all blocked waters will be awoken. This should be always be called on
> >>> + * software only fences, or alternatively be called after
> >>> + * dma_fence_ops::enable_signaling is called.
> >> I think we need to be cleare here when dma_fence_signal can be called:
> >> - for a sw-only fence (i.e. created with dma_fence_create)
> >> dma_fence_signal _must_ be called under all circumstances.
> >> - for any other fences, dma_fence_signal may be called, but it _must_ be
> >> called once the ->enable_signalling func has been called and return 0
> >> (i.e. success).
> >> - it may be called only _once_.
>
> As we discussed on irc it might be beneficial to be able to have it called
> twice, the second time would be a noop, however.
Agreed.
[snip]
> >>> + /* At this point, if enable_signaling returns any error
> >>> + * a wakeup has to be performanced regardless.
> >>> + * -ENOENT signals fence was already signaled. Any other
> >>> error
> >>> + * inidicates a catastrophic hardware error.
> >>> + *
> >>> + * If any hardware error occurs, nothing can be done against
> >>> + * it, so it's treated like the fence was already signaled.
> >>> + * No synchronization can be performed, so we have to assume
> >>> + * the fence was already signaled.
> >>> + */
> >>> + ret = fence->ops->enable_signaling(fence);
> >>> + if (ret) {
> >>> + signaled = true;
> >>> + dma_fence_signal(fence);
> >> I think we should call dma_fence_signal only for -ENOENT and pass all
> >> other errors back as-is. E.g. on -ENOMEM or so we might want to retry ...
>
> Also discussed on irc, boolean might be a better solution until we start
> dealing with
> hardware on fire. :) This would however likely be dealt in the same way as
> signaling,
> however.
Agreed.
[snip]
> >>> +
> >>> + if (!ret) {
> >>> + cb->base.flags = 0;
> >>> + cb->base.func = __dma_fence_wake_func;
> >>> + cb->base.private = priv;
> >>> + cb->fence = fence;
> >>> + cb->func = func;
> >>> + __add_wait_queue(&fence->event_queue, &cb->base);
> >>> + }
> >>> + spin_unlock_irqrestore(&fence->event_queue.lock, flags);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(dma_fence_add_callback);
> >> I think for api completenes we should also have a
> >> dma_fence_remove_callback function.
> > We did originally but Maarten found it was difficult to deal with
> > properly when the gpu's hang. I think his alternative was just to
> > require the hung driver to signal the fence. I had kicked around the
> > idea of a dma_fence_cancel() alternative to signal that could pass an
> > error thru to the waiting driver.. although not sure if the other
> > driver could really do anything differently at that point.
>
> No, there is a very real reason I removed dma_fence_remove_callback. It is
> absolutely non-trivial to cancel it once added, since you have to deal with
> all kinds of race conditions.. See i915_gem_reset_requests in my git tree:
> http://cgit.freedesktop.org/~mlankhorst/linux/commit/?id=673c4b2550bc63ec134bca47a95dabd617a689ce
I don't see the point in that code ... Why can't we drop the kref
_outside_ of the critical section protected by event_queue_lock? Then you
pretty much have an open-coded version of dma_fence_callback_cancel in
there.
> This is the only way to do it completely deadlock/memory corruption free
> since you essentially have a locking inversion to avoid. I had it wrong
> the first 2 times too, even when I knew about a lot of the locking
> complications. If you want to do it, in most cases it will likely be easier
> to just eat the signal and ignore it instead of canceling.
>
> >>> +
> >>> +/**
> >>> + * dma_fence_wait - wait for a fence to be signaled
> >>> + *
> >>> + * @fence: [in]The fence to wait on
> >>> + * @intr:[in]if true, do an interruptible wait
> >>> + * @timeout: [in]absolute time for timeout, in jiffies.
> >> I don't quite like this, I think we should keep the styl of all other
> >> wait_*_timeout functions and pass the arg as timeout in jiffies (and also
> >> the same return semantics). Otherwise well have funny code that needs to
> >> handle return values differently depending upon whether it waits upon a
> >> dma_fence or a native object (where it would us the wait_*_timeout
> >> functions directly).
> > We did start out this way, but there was an ugly jiffies roll
Re: [PATCH] drm: EDID quirk improvements
Dear Ian, Am Samstag, den 11.08.2012, 10:38 -0500 schrieb Ian Pilcher: > On 08/11/2012 03:31 AM, Paul Menzel wrote: > > As a side note, could you also mention the patch iteration in the tag, > > that means [PATCH vN] so that I know what is the latest version. That > > would be great. > > Can you (or anyone else reading this) point me to how to do this with > git send-email? No, but you can do that with `git format-patch` [3]. git format-patch --subject-prefix="PATCH v6" … > > I would submit adding the new quirk flags and the LG quirk as separate > > patches. If it is not too much work, it would be great if you could > > split them up. > > Does git provide any facility to make this easier? As far as I can > tell, the process is to start over with a newly cloned repository, > apply the current patch, manually back out the changes that I want to > separate, commit, manually redo the separate changes, and commit > again. Is that correct? Kind of. But you can do that in the same clone within your checkout/clone. Basically you go before your commit and then use `git add -p` to add only the hunks to the commit you need [4][5]. Do not worry that your changes get lost. `git reflog` still shows you the commit hash of your original patch. You can always go back to that by doing `git reset --hard ` or checking it out in another branch `git checkout -b myoldpatch_v5 `. You can read the manual with `git help commandname` like `git help add`. The folks in #git on irc.freenode.net are also very helpful. […] > >> +DEFINE_MUTEX(edid_quirk_list_mutex); > >> + > >> +/** > >> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for > >> printing > >> + * @mfg_id: the encoded manufacturer ID > >> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 > >> bytes) > > > > format*t*ed > > > > http://www.merriam-webster.com/dictionary/formatted > > Thanks for catching that. I keep finding little typos like that; very > annoying. Well, it was the only typo I found. I thought you are using a spell checker already. […] > I can't think of a way to provide anything useful within the scope of a > parameter description. Any suggestions? Maybe just say give an example `vendor:model:quirk`. > A URL would be great, but what would it be? (I don't have a personal > web site, and that doesn't seem really appropriate anyway.) http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c > > Thanks again for that great patch. With the comments addressed above you > > can add my acknowledgment. > > > > Acked-by: Paul Menzel > > Thank you for your feedback. > > Are you saying that I should add the acked-by? If so, how? (You can > probably tell that I'm really struggling with git.) That is very easy: `git commit --amend` to just edit the commit message. You can edit the other changes above, by editing the appropriate files and then doing `git commit --amend -a`. This adds all changes in files known to Git also to the last commit. > > I am going to try to test that patch too for a Philips and LG TV [2]. > > I hope it helps. We will see tomorrow. Thanks, Paul [3] http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotating_new_revision [4] http://stackoverflow.com/questions/4307095/git-how-to-split-up-a-commit-buried-in-history [5] http://git.661346.n2.nabble.com/How-to-split-a-big-commit-td6238260.html signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
ping: Re: quirk for Samsung 2443BW
Hello Adam,
On Fri, Jul 20, 2012 at 12:34:21PM +1000, Dave Airlie wrote:
> > Samsung 2443BW is 1920x1200 but reports 1920x1080 in the EDID. Attached
> > is a proof-of-concept implementation of a quirk. It works on my i686 PC.
...
> > This implementation matches the wrong mode by size. Other approaches are
> > possible.
> >
> > I'd appreciate feedback.
>
> ajax, seem sane?
Ping?
With kind regards,
Baurzhan.
>From b6c7a43e8b8fa0b6b39ed4a99c463071269d1a50 Mon Sep 17 00:00:00 2001
From: Baurzhan Ismagulov
Date: Sat, 14 Jul 2012 22:27:18 +0200
Subject: [PATCH 1/2] drm: Make edid_quirk_list const
Signed-off-by: Baurzhan Ismagulov
---
drivers/gpu/drm/drm_edid.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..09ff2bb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -86,7 +86,7 @@ static struct edid_quirk {
char vendor[4];
int product_id;
u32 quirks;
-} edid_quirk_list[] = {
+} const edid_quirk_list[] = {
/* Acer AL1706 */
{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
/* Acer F51 */
@@ -415,7 +415,7 @@ EXPORT_SYMBOL(drm_get_edid);
*
* Returns true if @vendor is in @edid, false otherwise
*/
-static bool edid_vendor(struct edid *edid, char *vendor)
+static bool edid_vendor(struct edid *edid, const char *vendor)
{
char edid_vendor[3];
@@ -435,7 +435,7 @@ static bool edid_vendor(struct edid *edid, char *vendor)
*/
static u32 edid_get_quirks(struct edid *edid)
{
- struct edid_quirk *quirk;
+ const struct edid_quirk *quirk;
int i;
for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
--
1.7.2.5
>From ea4ca18f607c3829239ad602b0cb8d319fbcd75e Mon Sep 17 00:00:00 2001
From: Baurzhan Ismagulov
Date: Sat, 14 Jul 2012 22:23:33 +0200
Subject: [PATCH 2/2] drm: Add quirk for Samsung SyncMaster 2443BW
Signed-off-by: Baurzhan Ismagulov
---
drivers/gpu/drm/drm_edid.c | 64 ++-
1 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 09ff2bb..73dda54 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -68,12 +68,14 @@
#define EDID_QUIRK_DETAILED_SYNC_PP (1 << 6)
/* Force reduced-blanking timings for detailed modes */
#define EDID_QUIRK_FORCE_REDUCED_BLANKING (1 << 7)
+/* Force size */
+#define EDID_QUIRK_FORCE_SIZE (1 << 8)
struct detailed_mode_closure {
struct drm_connector *connector;
struct edid *edid;
bool preferred;
- u32 quirks;
+ const struct edid_quirk *quirk;
int modes;
};
@@ -82,10 +84,23 @@ struct detailed_mode_closure {
#define LEVEL_GTF2 2
#define LEVEL_CVT 3
+struct size {
+ int x;
+ int y;
+};
+
+struct force_size {
+ struct size bad;
+ struct size good;
+};
+
static struct edid_quirk {
char vendor[4];
int product_id;
u32 quirks;
+ union {
+ struct force_size size;
+ } u;
} const edid_quirk_list[] = {
/* Acer AL1706 */
{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
@@ -122,6 +137,9 @@ static struct edid_quirk {
/* Samsung SyncMaster 22[5-6]BW */
{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
+ /* Samsung SyncMaster 2443BW */
+ { "SAM", 0x06b0, EDID_QUIRK_FORCE_SIZE,
+ .u.size = { { 1920, 1080 }, { 1920, 1200 } } },
/* ViewSonic VA2026w */
{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
@@ -428,12 +446,12 @@ static bool edid_vendor(struct edid *edid, const char *vendor)
}
/**
- * edid_get_quirks - return quirk flags for a given EDID
+ * edid_get_quirk - return quirk data for a given EDID
* @edid: EDID to process
*
* This tells subsequent routines what fixes they need to apply.
*/
-static u32 edid_get_quirks(struct edid *edid)
+static const struct edid_quirk *edid_get_quirk(struct edid *edid)
{
const struct edid_quirk *quirk;
int i;
@@ -443,10 +461,10 @@ static u32 edid_get_quirks(struct edid *edid)
if (edid_vendor(edid, quirk->vendor) &&
(EDID_PRODUCT_ID(edid) == quirk->product_id))
- return quirk->quirks;
+ return quirk;
}
- return 0;
+ return NULL;
}
#define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -866,7 +884,7 @@ drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
struct edid *edid,
struct detailed_timing *timing,
- u32 quirks)
+ const struct edid_quirk *quirk)
{
struct drm_display_mode *mode;
struct detailed_pixel_timing *pt = &timing->data.pixel_data;
@@ -898,7 +916,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
return NULL;
}
- if (quirks & EDID_QUIRK_FORCE_REDUCED_BLANKING) {
+ if (quirk && quirk->quirks & EDID_QUIRK_FORCE_REDUCED_BLANKING) {
mode = drm_cvt_mode(dev, hactive, vactive, 60, true, false, false);
if (!mode)
return NULL;
@@ -906,11 +924,17 @@ static struct drm_display_mode *drm_mode_detailed(
Re: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 2:22 PM, Daniel Vetter wrote: >> >> + >> >> +/** >> >> + * dma_fence_wait - wait for a fence to be signaled >> >> + * >> >> + * @fence: [in]The fence to wait on >> >> + * @intr:[in]if true, do an interruptible wait >> >> + * @timeout: [in]absolute time for timeout, in jiffies. >> > >> > I don't quite like this, I think we should keep the styl of all other >> > wait_*_timeout functions and pass the arg as timeout in jiffies (and also >> > the same return semantics). Otherwise well have funny code that needs to >> > handle return values differently depending upon whether it waits upon a >> > dma_fence or a native object (where it would us the wait_*_timeout >> > functions directly). >> >> We did start out this way, but there was an ugly jiffies roll-over >> problem that was difficult to deal with properly. Using an absolute >> time avoided the problem. > > Well, as-is the api works differently than all the other _timeout apis > I've seen in the kernel, which makes it confusing. Also, I don't quite see > what jiffies wraparound issue there is? iirc, the problem was in dmabufmgr, in dmabufmgr_wait_completed_cpu().. with an absolute timeout, it could loop over all the fences without having to adjust the timeout for the elapsed time. Otherwise it had to adjust the timeout and keep track of when the timeout elapsed without confusing itself via rollover. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/2] Enhanced EDID quirk functionality
Updated patch set, based on Paul's feedback. * Separate user-defined quirks stuff from new HDMI-related quirks * (Hopefully) improve documentation Also continuing to explore the wonders of git format-patch and git send-email. Ian Pilcher (2): drm: Add user-defined EDID quirks capability drm: Add EDID quirks to disable HDMI audio and InfoFrames Documentation/EDID/edid_quirks.txt | 126 + drivers/gpu/drm/drm_drv.c | 2 + drivers/gpu/drm/drm_edid.c | 524 + drivers/gpu/drm/drm_stub.c | 6 + drivers/gpu/drm/drm_sysfs.c| 19 ++ include/drm/drmP.h | 10 + include/drm/drm_edid.h | 13 +- 7 files changed, 639 insertions(+), 61 deletions(-) create mode 100644 Documentation/EDID/edid_quirks.txt -- 1.7.11.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/2] drm: Add user-defined EDID quirks capability
Add the ability for users to define their own EDID quirks via a
module parameter or sysfs attribute.
Signed-off-by: Ian Pilcher
Acked-by: Paul Menzel
---
Documentation/EDID/edid_quirks.txt | 126 ++
drivers/gpu/drm/drm_drv.c | 2 +
drivers/gpu/drm/drm_edid.c | 500 -
drivers/gpu/drm/drm_stub.c | 6 +
drivers/gpu/drm/drm_sysfs.c| 19 ++
include/drm/drmP.h | 10 +
include/drm/drm_edid.h | 13 +-
7 files changed, 615 insertions(+), 61 deletions(-)
create mode 100644 Documentation/EDID/edid_quirks.txt
diff --git a/Documentation/EDID/edid_quirks.txt
b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 000..0c9c746
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,126 @@
+ EDID Quirks
+ =
+ Ian Pilcher
+August 11, 2012
+
+
+"EDID blocks out in the wild have a variety of bugs"
+-- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of "quirk flags." The kernel
+includes a variety of built-in quirks. (They are stored in the edid_quirk_list
+array in drivers/gpu/drm/drm_edid.c.)
+
+An example of a built-in EDID quirk is:
+
+ACR:0xad46:0x0001
+
+The first field is the manufacturer ID (Acer, Inc.), the second field is the
+manufacturer's product code, and the third field contains the quirk flags for
+that display type.
+
+The quirk flags are defined in drivers/gpu/drm/drm_edid.c. Each flag has a
+symbolic name beginning with EDID_QUIRK_, along with a numerical value. Each
+flag should also have an associated comment which provides an idea of its
+effect. Note that the values in the source file are expressed as bit
shifts[2]:
+
+* 1 << 0: 0x0001
+* 1 << 1: 0x0002
+* 1 << 2: 0x0004
+* etc.
+
+
+sysfs interface
+===
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks:
+
+# cat /sys/class/drm/edid_quirks
+ ACR:0xad46:0x0001
+ API:0x7602:0x0001
+ ACR:0x0977:0x0020
+0x9e6a:0x077e:0x0080
+...
+
+("Nonconformant" manufacturer IDs are displayed as hexadecimal values.)
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size. This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks. The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to /sys/class/drm/edid_quirks:
+
+# echo FOO:0x:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically. (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+# echo 0x19ef:0x:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+# echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+# echo FOO:0x:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the slot occupied by that quirk available.
+
+# echo FOO:0x:0 > /sys/class/drm/edid_quirks
+
+Writing an "at symbol" (@) clears the entire quirk list:
+
+# echo @ > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+# echo @,FOO:0x:0x100,BAR:0x:0x001,BAZ:0x:0x002 > \
+/sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made. For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line). The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference. When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the paramet
[PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames
Add EDID quirk flags to disable HDMI audio and HDMI InfoFrames.
Add quirk for LG L246WP.
Signed-off-by: Ian Pilcher
Acked-by: Paul Menzel
---
drivers/gpu/drm/drm_edid.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bb3ba20..6c143ed 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -70,6 +70,10 @@
#define EDID_QUIRK_DETAILED_SYNC_PP(1 << 6)
/* Force reduced-blanking timings for detailed modes */
#define EDID_QUIRK_FORCE_REDUCED_BLANKING (1 << 7)
+/* Display is confused by InfoFrames; don't sent any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES (1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO(1 << 9)
struct detailed_mode_closure {
struct drm_connector *connector;
@@ -156,6 +160,10 @@ union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+ /* LG L246WP */
+ { { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+ EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
/*
* When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
* provide some room for user-supplied quirks.
@@ -2109,6 +2117,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
int i, hdmi_id;
int start_offset, end_offset;
bool is_hdmi = false;
+ char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+ if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+ DRM_INFO("Disabling HDMI InfoFrames on display %s "
+"due to EDID quirk\n",
+drm_edid_display_id_format(edid->display_id, buf, 1));
+ goto end;
+ }
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
@@ -2157,6 +2173,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
int i, j;
bool has_audio = false;
int start_offset, end_offset;
+ char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+ if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+ DRM_INFO("Disabling HDMI audio on display %s "
+"due to EDID quirk\n",
+drm_edid_display_id_format(edid->display_id, buf, 1));
+ goto end;
+ }
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
--
1.7.11.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: EDID quirk improvements
Dear Ian, thank you for your patch! The best thing is, I just asked Adam on #intel-gfx the other day, if it was possible to have infrastructure to test quirks without having to patch and build the Linux kernel. Nice! As a side note, could you also mention the patch iteration in the tag, that means [PATCH vN] so that I know what is the latest version. That would be great. Am Freitag, den 10.08.2012, 13:44 -0500 schrieb Ian Pilcher: > Add ability for user to add or remove EDID quirks, via module > parameter or sysfs. Also add two new quirk flags -- > EDID_QUIRK_DISABLE_INFOFRAMES and EDID_QUIRK_NO_AUDIO -- and adds > a quirk for the LG L246WP display. I would submit adding the new quirk flags and the LG quirk as separate patches. If it is not too much work, it would be great if you could split them up. > Document module parameter and sysfs interface. > --- > Documentation/EDID/edid_quirks.txt | 161 +++ > drivers/gpu/drm/drm_drv.c | 2 + > drivers/gpu/drm/drm_edid.c | 527 > + > drivers/gpu/drm/drm_stub.c | 5 + > drivers/gpu/drm/drm_sysfs.c| 19 ++ > include/drm/drmP.h | 10 + > include/drm/drm_edid.h | 13 +- > 7 files changed, 676 insertions(+), 61 deletions(-) > create mode 100644 Documentation/EDID/edid_quirks.txt > > diff --git a/Documentation/EDID/edid_quirks.txt > b/Documentation/EDID/edid_quirks.txt > new file mode 100644 > index 000..256ded0 > --- /dev/null > +++ b/Documentation/EDID/edid_quirks.txt > @@ -0,0 +1,161 @@ > + EDID Quirks > + = > + Ian Pilcher > + August 8, 2012 > + > + > +"EDID blocks out in the wild have a variety of bugs" > +-- from drivers/gpu/drm/drm_edid.c > + > + > +Overview > + > + > +EDID quirks provide a mechanism for working around display hardware with > buggy > +EDID data. > + > +An individual EDID quirk maps a display type (identified by its EDID > +manufacturer ID and product code[1]) to a set of flags. For example, the > current > +list of quirks built into the kernel is: > + > +ACR:0xad46:0x0001 > +API:0x7602:0x0001 > +ACR:0x0977:0x0020 > +MAX:0x05ec:0x0001 > +MAX:0x077e:0x0001 > +EPI:0xe780:0x0002 > +EPI:0x2028:0x0001 > +FCM:0x3520:0x000c > +LPL:0x:0x0010 > +LPL:0x2a00:0x0010 > +PHL:0xe014:0x0020 > +PTS:0x02fd:0x0020 > +SAM:0x021d:0x0040 > +SAM:0x0254:0x0001 > +SAM:0x027e:0x0001 > +VSC:0x139c:0x0080 > +GSM:0x563f:0x0300 > + > +The first field of each quirk is the manufacturer ID, the second field is the > +product code, and the third field is the quirk flags. > + > +NOTE: All of the manufacturer IDs above are displayed as 3-character strings, > +because they are conformant IDs that have been properly encoded: > + > +- The most-significant bit of the encoded ID is 0 > +- They only contain ASCII characters in the range A-Z > + > +IDs that do not conform to these rules are displayed as "raw" hexadecimal > +values. > + > +The current quirk flags are: > + > +/* First detailed mode wrong, use largest 60Hz mode */ > +#define EDID_QUIRK_PREFER_LARGE_60 0x0001 > + > +/* Reported 135MHz pixel clock is too high, needs adjustment */ > +#define EDID_QUIRK_135_CLOCK_TOO_HIGH 0x0002 > + > +/* Prefer the largest mode at 75 Hz */ > +#define EDID_QUIRK_PREFER_LARGE_75 0x0004 > + > +/* Detail timing is in cm not mm */ > +#define EDID_QUIRK_DETAILED_IN_CM 0x0008 > + > +/* Detailed timing descriptors have bogus size values, so just take the > + * maximum size and use that. > + */ > +#define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE0x0010 > + > +/* Monitor forgot to set the first detailed is preferred bit. */ > +#define EDID_QUIRK_FIRST_DETAILED_PREFERRED 0x0020 > + > +/* use +hsync +vsync for detailed mode */ > +#define EDID_QUIRK_DETAILED_SYNC_PP 0x0040 > + > +/* Force reduced-blanking timings for detailed modes */ > +#define EDID_QUIRK_FORCE_REDUCED_BLANKING 0x0080 You even added this one from the following commit. commit bc42aabc6a01b92b0f961d65671564e0e1cd7592 Author: Adam Jackson Date: Wed May 23 16:26:54 2012 -0400 drm/edid/quirks: ViewSonic VA2026w I am going to need that quirk. Great! > + > +/* Display is confused by InfoFrames; don't sent any */ > +#define EDID_QUIRK_DISABLE_INFOFRAMES 0x0100 > + > +/* Display doesn't have any audio output */ > +#define EDID_QUIRK_NO_AUDIO 0x0200 > + > + > +sysfs interface > +=== > + >
[PATCH V5 11/18] drm/radeon: Include swiotlb.h if SWIOTLB configured.
Loongson has SWIOTLB configured, if without this patch kernel compilation fails. Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/radeon/radeon_ttm.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 5b71c71..fc3ac22 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -41,6 +41,10 @@ #include "radeon_reg.h" #include "radeon.h" +#ifdef CONFIG_SWIOTLB +#include +#endif + #define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT) static int radeon_ttm_debugfs_init(struct radeon_device *rdev); -- 1.7.7.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V5 12/18] drm: Handle io prot correctly for MIPS.
Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/drm_vm.c |2 +- drivers/gpu/drm/ttm/ttm_bo_util.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index 961ee08..3f06166 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct vm_area_struct *vma) tmp = pgprot_writecombine(tmp); else tmp = pgprot_noncached(tmp); -#elif defined(__sparc__) || defined(__arm__) +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__) tmp = pgprot_noncached(tmp); #endif return tmp; diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index f8187ea..0df71ea 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -472,7 +472,7 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) else tmp = pgprot_noncached(tmp); #endif -#if defined(__sparc__) +#if defined(__sparc__) || defined(__mips__) if (!(caching_flags & TTM_PL_FLAG_CACHED)) tmp = pgprot_noncached(tmp); #endif -- 1.7.7.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH V5 13/18] drm: Define SAREA_MAX for Loongson (PageSize = 16KB).
Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel@lists.freedesktop.org --- include/drm/drm_sarea.h |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/drm/drm_sarea.h b/include/drm/drm_sarea.h index ee5389d..1d1a858 100644 --- a/include/drm/drm_sarea.h +++ b/include/drm/drm_sarea.h @@ -37,6 +37,8 @@ /* SAREA area needs to be at least a page */ #if defined(__alpha__) #define SAREA_MAX 0x2000U +#elif defined(__mips__) +#define SAREA_MAX 0x4000U #elif defined(__ia64__) #define SAREA_MAX 0x1U /* 64kB */ #else -- 1.7.7.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] gma500: Consider CRTC initially active.
From: Forest Bond This causes the pipe to be forced off prior to initial mode set, which roughly mirrors the behavior of the i915 driver. It fixes initial mode setting on my Intel DN2800MT (Cedarview) board. Without it, mode setting triggers an out-of-range error from the monitor for most modes, but only on initial configuration (i.e. they can be configured successfully from userspace after that). Signed-off-by: Forest Bond --- drivers/gpu/drm/gma500/psb_intel_display.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index 30dc22a..8033526 100644 --- a/drivers/gpu/drm/gma500/psb_intel_display.c +++ b/drivers/gpu/drm/gma500/psb_intel_display.c @@ -1362,6 +1362,9 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe, (struct drm_connector **) (psb_intel_crtc + 1); psb_intel_crtc->mode_set.num_connectors = 0; psb_intel_cursor_init(dev, psb_intel_crtc); + + /* Set to true so that the pipe is forced off on initial config. */ + psb_intel_crtc->active = true; } int psb_intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, -- 1.7.0.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] gma500: Fix comment mispelling in cdv_intel_limits definition.
From: Forest Bond
Signed-off-by: Forest Bond
---
drivers/gpu/drm/gma500/cdv_intel_display.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c
b/drivers/gpu/drm/gma500/cdv_intel_display.c
index a68509b..883a9f3 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -65,7 +65,7 @@ struct cdv_intel_limit_t {
#define CDV_LIMIT_DAC_HDMI_96 3
static const struct cdv_intel_limit_t cdv_intel_limits[] = {
- { /* CDV_SIGNLE_LVDS_96MHz */
+ { /* CDV_SINGLE_LVDS_96MHz */
.dot = {.min = 2, .max = 115500},
.vco = {.min = 180, .max = 360},
.n = {.min = 2, .max = 6},
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] gma: psb_intel_crtc: Drop crtc_enable flag.
From: Forest Bond
This is set when setting DPMS on and off, but it isn't checked anywhere,
so just remove it.
Signed-off-by: Forest Bond
---
drivers/gpu/drm/gma500/cdv_intel_display.c |2 --
drivers/gpu/drm/gma500/psb_intel_drv.h |1 -
2 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c
b/drivers/gpu/drm/gma500/cdv_intel_display.c
index 883a9f3..2100aff 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -841,7 +841,6 @@ static void cdv_intel_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* Give the overlay scaler a chance to enable
* if it's on this pipe */
/* psb_intel_crtc_dpms_video(crtc, true); TODO */
- psb_intel_crtc->crtc_enable = true;
break;
case DRM_MODE_DPMS_OFF:
if (!psb_intel_crtc->active)
@@ -893,7 +892,6 @@ static void cdv_intel_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* Wait for the clocks to turn off. */
udelay(150);
cdv_intel_update_watermark(dev, crtc);
- psb_intel_crtc->crtc_enable = false;
break;
}
/*Set FIFO Watermarks*/
diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h
b/drivers/gpu/drm/gma500/psb_intel_drv.h
index ebe1a28..e179c36 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -190,7 +190,6 @@ struct psb_intel_crtc {
u32 mode_flags;
bool active;
- bool crtc_enable;
/* Saved Crtc HW states */
struct psb_intel_crtc_state *crtc_state;
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Kconfig: warning: (DRM_RADEON_KMS && DRM_I915 && …) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT)
Dear Linux folks, where should I report the following warning warning: (DRM_RADEON_KMS && DRM_I915 && STUB_POULSBO && FB_BACKLIGHT && USB_APPLEDISPLAY && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10 && APPLE_GMUX) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) I get with the following commit? commit f4ba394c1b02e7fc2179fda8d3941a5b3b65efb6 Merge: bf44ce8 5d299f3 Author: Linus Torvalds Date: Wed Aug 8 20:06:43 2012 +0300 Thanks, Paul signature.asc Description: This is a digitally signed message part ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
>> A dma-fence can be attached to a buffer which is being filled or consumed
>> by hw, to allow userspace to pass the buffer without waiting to another
>> device. For example, userspace can call page_flip ioctl to display the
>> next frame of graphics after kicking the GPU but while the GPU is still
>> rendering. The display device sharing the buffer with the GPU would
>> attach a callback to get notified when the GPU's rendering-complete IRQ
>> fires, to update the scan-out address of the display, without having to
>> wake up userspace.
>>
>> A dma-fence is transient, one-shot deal. It is allocated and attached
>> to one or more dma-buf's. When the one that attached it is done, with
>> the pending operation, it can signal the fence.
>>
>> + dma_fence_signal()
>>
>> The dma-buf-mgr handles tracking, and waiting on, the fences associated
>> with a dma-buf.
>>
>> TODO maybe need some helper fxn for simple devices, like a display-
>> only drm/kms device which simply wants to wait for exclusive fence to
>> be signaled, and then attach a non-exclusive fence while scanout is in
>> progress.
>>
>> The one pending on the fence can add an async callback:
>> + dma_fence_add_callback()
>> The callback can optionally be cancelled with remove_wait_queue()
>>
>> Or wait synchronously (optionally with timeout or interruptible):
>> + dma_fence_wait()
>>
>> A default software-only implementation is provided, which can be used
>> by drivers attaching a fence to a buffer when they have no other means
>> for hw sync. But a memory backed fence is also envisioned, because it
>> is common that GPU's can write to, or poll on some memory location for
>> synchronization. For example:
>>
>> fence = dma_buf_get_fence(dmabuf);
>> if (fence->ops == &bikeshed_fence_ops) {
>> dma_buf *fence_buf;
>> dma_bikeshed_fence_get_buf(fence, &fence_buf, &offset);
>> ... tell the hw the memory location to wait on ...
>> } else {
>> /* fall-back to sw sync * /
>> dma_fence_add_callback(fence, my_cb);
>> }
>>
>> On SoC platforms, if some other hw mechanism is provided for synchronizing
>> between IP blocks, it could be supported as an alternate implementation
>> with it's own fence ops in a similar way.
>>
>> To facilitate other non-sw implementations, the enable_signaling callback
>> can be used to keep track if a device not supporting hw sync is waiting
>> on the fence, and in this case should arrange to call dma_fence_signal()
>> at some point after the condition has changed, to notify other devices
>> waiting on the fence. If there are no sw waiters, this can be skipped to
>> avoid waking the CPU unnecessarily. The handler of the enable_signaling
>> op should take a refcount until the fence is signaled, then release its ref.
>>
>> The intention is to provide a userspace interface (presumably via eventfd)
>> later, to be used in conjunction with dma-buf's mmap support for sw access
>> to buffers (or for userspace apps that would prefer to do their own
>> synchronization).
>
> I think the commit message should be cleaned up: Kill the TODO, rip out
> the bikeshed_fence and otherwise update it to the latest code.
>
>>
>> v1: Original
>> v2: After discussion w/ danvet and mlankhorst on #dri-devel, we decided
>> that dma-fence didn't need to care about the sw->hw signaling path
>> (it can be handled same as sw->sw case), and therefore the fence->ops
>> can be simplified and more handled in the core. So remove the signal,
>> add_callback, cancel_callback, and wait ops, and replace with a simple
>> enable_signaling() op which can be used to inform a fence supporting
>> hw->hw signaling that one or more devices which do not support hw
>> signaling are waiting (and therefore it should enable an irq or do
>> whatever is necessary in order that the CPU is notified when the
>> fence is passed).
>> v3: Fix locking fail in attach_fence() and get_fence()
>> v4: Remove tie-in w/ dma-buf.. after discussion w/ danvet and mlankorst
>> we decided that we need to be able to attach one fence to N dma-buf's,
>> so using the list_head in dma-fence struct would be problematic.
>> v5: [ Maarten Lankhorst ] Updated for dma-bikeshed-fence and dma-buf-manager.
>> v6: [ Maarten Lankhorst ] I removed dma_fence_cancel_callback and some
>> comments
>> about checking if fence fired or not. This is broken by design.
>> waitqueue_active during destruction is now fatal, since the signaller
>> should be holding a reference in enable_signalling until it signalled
>> the fence. Pass the original dma_fence_cb along, and call __remove_wait
>> in the dma_fence_callback handler, so that no cleanup needs to be
>> performed.
>> v7: [ Maarten Lankhorst ] Set cb->func and only enable sw signaling if
>> fence wasn't signaled yet, for example for hardware fence
[Bug 45018] [bisected] rendering regression and va conflicts since added support for virtual address space on cayman v11
https://bugs.freedesktop.org/show_bug.cgi?id=45018 --- Comment #118 from Alexandre Demers 2012-08-11 04:49:31 UTC --- Reproduced again with exactly the setup Alex told me to use (kernel 3.6-rc1+Jerome's patch v4 and latest mesa containing Christian's fix). To reproduce, I clicked repeatedly on Activities on top left corner of Gnome shell until it locked: Everything.log --- Aug 11 00:23:08 Xander kernel: [92926.580673] radeon :01:00.0: offset 0x20 is in reserved area 0x80 Aug 11 00:23:08 Xander kernel: [92926.587281] [drm:radeon_cs_parser_relocs] *ERROR* gem object lookup failed 0x11 Aug 11 00:23:08 Xander kernel: [92926.587291] [drm:radeon_cs_ioctl] *ERROR* Failed to parse relocation -2! Aug 11 00:23:08 Xander kernel: [92926.597151] radeon :01:00.0: offset 0x20 is in reserved area 0x80 Aug 11 00:23:18 Xander kernel: [92937.073091] radeon :01:00.0: GPU lockup CP stall for more than 1msec Aug 11 00:23:18 Xander kernel: [92937.073105] radeon :01:00.0: GPU lockup (waiting for 0x0009ea1d last fence id 0x0009ea1c) Aug 11 00:23:18 Xander kernel: [92937.074236] radeon :01:00.0: Saved 15 dwords of commands on ring 0. Aug 11 00:23:18 Xander kernel: [92937.074243] radeon :01:00.0: GPU softreset Aug 11 00:23:18 Xander kernel: [92937.074248] radeon :01:00.0: GRBM_STATUS=0xF5700828 Aug 11 00:23:18 Xander kernel: [92937.074253] radeon :01:00.0: GRBM_STATUS_SE0=0xFC01 Aug 11 00:23:18 Xander kernel: [92937.074258] radeon :01:00.0: GRBM_STATUS_SE1=0xFC01 Aug 11 00:23:18 Xander kernel: [92937.074263] radeon :01:00.0: SRBM_STATUS=0x20020FC0 Aug 11 00:23:18 Xander kernel: [92937.074269] radeon :01:00.0: R_008674_CP_STALLED_STAT1 = 0x Aug 11 00:23:18 Xander kernel: [92937.074274] radeon :01:00.0: R_008678_CP_STALLED_STAT2 = 0x4000 Aug 11 00:23:18 Xander kernel: [92937.074279] radeon :01:00.0: R_00867C_CP_BUSY_STAT = 0x8004 Aug 11 00:23:18 Xander kernel: [92937.074284] radeon :01:00.0: R_008680_CP_STAT = 0x80228647 Aug 11 00:23:18 Xander kernel: [92937.074289] radeon :01:00.0: VM_CONTEXT0_PROTECTION_FAULT_ADDR 0x00074124 Aug 11 00:23:18 Xander kernel: [92937.074294] radeon :01:00.0: VM_CONTEXT0_PROTECTION_FAULT_STATUS 0x00071001 Aug 11 00:23:18 Xander kernel: [92937.074300] radeon :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0x21F1 Aug 11 00:23:18 Xander kernel: [92937.074305] radeon :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x020A4004 Aug 11 00:23:19 Xander kernel: [92937.223150] radeon :01:00.0: Wait for MC idle timedout ! Aug 11 00:23:19 Xander kernel: [92937.223152] radeon :01:00.0: GRBM_SOFT_RESET=0xDF7B Aug 11 00:23:19 Xander kernel: [92937.223254] radeon :01:00.0: GRBM_STATUS=0x80103828 Aug 11 00:23:19 Xander kernel: [92937.223256] radeon :01:00.0: GRBM_STATUS_SE0=0x0407 Aug 11 00:23:19 Xander kernel: [92937.223257] radeon :01:00.0: GRBM_STATUS_SE1=0x0407 Aug 11 00:23:19 Xander kernel: [92937.223258] radeon :01:00.0: SRBM_STATUS=0x20020FC0 Aug 11 00:23:19 Xander kernel: [92937.223260] radeon :01:00.0: R_008674_CP_STALLED_STAT1 = 0x Aug 11 00:23:19 Xander kernel: [92937.223262] radeon :01:00.0: R_008678_CP_STALLED_STAT2 = 0x Aug 11 00:23:19 Xander kernel: [92937.223263] radeon :01:00.0: R_00867C_CP_BUSY_STAT = 0x Aug 11 00:23:19 Xander kernel: [92937.223264] radeon :01:00.0: R_008680_CP_STAT = 0x Aug 11 00:23:19 Xander kernel: [92937.224266] radeon :01:00.0: GPU reset succeeded, trying to resume Aug 11 00:23:19 Xander kernel: [92937.230003] [drm] probing gen 2 caps for device 1022:9603 = 2/0 Aug 11 00:23:19 Xander kernel: [92937.230004] [drm] enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0 Aug 11 00:23:19 Xander kernel: [92937.388426] radeon :01:00.0: Wait for MC idle timedout ! Aug 11 00:23:19 Xander kernel: [92937.546743] radeon :01:00.0: Wait for MC idle timedout ! Aug 11 00:23:19 Xander kernel: [92937.548662] [drm] PCIE GART of 512M enabled (table at 0x0004). Aug 11 00:23:19 Xander kernel: [92937.548751] radeon :01:00.0: WB enabled Aug 11 00:23:19 Xander kernel: [92937.548754] radeon :01:00.0: fence driver on ring 0 use gpu addr 0x4c00 and cpu addr 0x88022332dc00 Aug 11 00:23:19 Xander kernel: [92937.548755] radeon :01:00.0: fence driver on ring 1 use gpu addr 0x4c04 and cpu addr 0x88022332dc04 Aug 11 00:23:19 Xander kernel: [92937.548757] radeon :01:00.0: fence driver on ring 2 use gpu addr 0x4c08 and cpu addr 0x88022332dc08 Aug 11 00:23:19 Xander kernel: [92937.752374] [drm:r600_ring_test] *ERROR* radeon: ring 0 test failed (scratch(0x8504)=0xCAFEDEAD) Aug 11 00:23:19 Xander kernel: [92937.752377] [drm:cayman_resume] *ERROR* cayman startup failed on resume Could it be a previously hidden bug that patches from Jerome an
[PATCH] drm: EDID quirk improvements
mes on display %s "
> + "due to EDID quirk\n",
> + drm_edid_display_id_format(edid->display_id, buf, 1));
> + goto end;
> + }
>
> edid_ext = drm_find_cea_extension(edid);
> if (!edid_ext)
> @@ -1771,6 +2176,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
> int i, j;
> bool has_audio = false;
> int start_offset, end_offset;
> + char buf[EDID_DISPLAY_ID_BUF_SIZE];
> +
> + if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
> + DRM_INFO("Disabling HDMI audio on display %s "
> + "due to EDID quirk\n",
> + drm_edid_display_id_format(edid->display_id, buf, 1));
> + goto end;
> + }
>
> edid_ext = drm_find_cea_extension(edid);
> if (!edid_ext)
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 21bcd4a..1885fc9 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -46,16 +46,21 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
> unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */
> EXPORT_SYMBOL(drm_timestamp_precision);
>
> +char *drm_edid_quirks = NULL;
> +EXPORT_SYMBOL(drm_edid_quirks);
> +
> MODULE_AUTHOR(CORE_AUTHOR);
> MODULE_DESCRIPTION(CORE_DESC);
> MODULE_LICENSE("GPL and additional rights");
> MODULE_PARM_DESC(debug, "Enable debug output");
> MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable
> [msecs]");
> MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps
> [usecs]");
> +MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt");
Not all users have access to the Linux source tree, so maybe a small
overview is still needed? Or even an URL?
>
> module_param_named(debug, drm_debug, int, 0600);
> module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
> module_param_named(timestamp_precision_usec, drm_timestamp_precision, int,
> 0600);
> +module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
>
> struct idr drm_minors_idr;
>
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 45ac8d6..84dc365 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
> __stringify(CORE_PATCHLEVEL) " "
> CORE_DATE);
>
> +static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
> +
> +static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
> + drm_edid_quirks_store);
> +
> /**
> * drm_sysfs_create - create a struct drm_sysfs_class structure
> * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
> @@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner,
> char *name)
> if (err)
> goto err_out_class;
>
> + err = class_create_file(class, &class_attr_edid_quirks_size);
> + if (err)
> + goto err_out_version;
> +
> + err = class_create_file(class, &class_attr_edid_quirks);
> + if (err)
> + goto err_out_quirks_size;
> +
> class->devnode = drm_devnode;
>
> return class;
>
> +err_out_quirks_size:
> + class_remove_file(class, &class_attr_edid_quirks_size);
> +err_out_version:
> + class_remove_file(class, &class_attr_version.attr);
> err_out_class:
> class_destroy(class);
> err_out:
> @@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
> {
> if ((drm_class == NULL) || (IS_ERR(drm_class)))
> return;
> + class_remove_file(drm_class, &class_attr_edid_quirks);
> + class_remove_file(drm_class, &class_attr_edid_quirks_size);
> class_remove_file(drm_class, &class_attr_version.attr);
> class_destroy(drm_class);
> drm_class = NULL;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d6b67bb..c947f3e 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
>
> extern unsigned int drm_vblank_offdelay;
> extern unsigned int drm_timestamp_precision;
> +extern char *drm_edid_quirks;
>
> extern struct class *drm_class;
> extern struct proc_dir_entry *drm_proc_root;
> @@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
> void drm_gem_vm_close(struct vm_area_struct *vma);
> int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>
> + /* EDID support (drm_edid.c) */
> +void drm_edid_quirks_param_process(void);
> +ssize_t drm_edid_quirks_size_show(struct class *class,
> + struct class_attribute *attr, char *buf);
> +ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute
> *attr,
> + char *buf);
> +ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute
> *attr,
> + const char *buf, size_t count);
> +
> #include "drm_global.h"
>
> static inline void
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 0cac551..713229b 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -202,11 +202,18 @@ struct detailed_timing {
> #define DRM_EDID_FEATURE_PM_SUSPEND (1 << 6)
> #define DRM_EDID_FEATURE_PM_STANDBY (1 << 7)
>
> +union edid_display_id {
> + struct {
> + __be16 mfg_id;
> + __le16 prod_code;
> + } __attribute__((packed)) s;
> + u32 u;
> +};
> +
> struct edid {
> u8 header[8];
> /* Vendor & product info */
> - u8 mfg_id[2];
> - u8 prod_code[2];
> + union edid_display_id display_id;
> u32 serial; /* FIXME: byte order */
> u8 mfg_week;
> u8 mfg_year;
> @@ -242,8 +249,6 @@ struct edid {
> u8 checksum;
> } __attribute__((packed));
>
> -#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
> -
> struct drm_encoder;
> struct drm_connector;
> struct drm_display_mode;
Thanks again for that great patch. With the comments addressed above you
can add my acknowledgment.
Acked-by: Paul Menzel
I am going to try to test that patch too for a Philips and LG TV [2].
Thanks,
Paul
[2] https://bugs.freedesktop.org/show_bug.cgi?id=26294
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/f5056002/attachment-0001.pgp>
[PATCH V5 11/18] drm/radeon: Include swiotlb.h if SWIOTLB configured.
Loongson has SWIOTLB configured, if without this patch kernel compilation fails. Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/radeon/radeon_ttm.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 5b71c71..fc3ac22 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -41,6 +41,10 @@ #include "radeon_reg.h" #include "radeon.h" +#ifdef CONFIG_SWIOTLB +#include +#endif + #define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT) static int radeon_ttm_debugfs_init(struct radeon_device *rdev); -- 1.7.7.3
[PATCH V5 12/18] drm: Handle io prot correctly for MIPS.
Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel at lists.freedesktop.org --- drivers/gpu/drm/drm_vm.c |2 +- drivers/gpu/drm/ttm/ttm_bo_util.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index 961ee08..3f06166 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -62,7 +62,7 @@ static pgprot_t drm_io_prot(uint32_t map_type, struct vm_area_struct *vma) tmp = pgprot_writecombine(tmp); else tmp = pgprot_noncached(tmp); -#elif defined(__sparc__) || defined(__arm__) +#elif defined(__sparc__) || defined(__arm__) || defined(__mips__) tmp = pgprot_noncached(tmp); #endif return tmp; diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index f8187ea..0df71ea 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -472,7 +472,7 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp) else tmp = pgprot_noncached(tmp); #endif -#if defined(__sparc__) +#if defined(__sparc__) || defined(__mips__) if (!(caching_flags & TTM_PL_FLAG_CACHED)) tmp = pgprot_noncached(tmp); #endif -- 1.7.7.3
[PATCH V5 13/18] drm: Define SAREA_MAX for Loongson (PageSize = 16KB).
Signed-off-by: Huacai Chen Signed-off-by: Hongliang Tao Signed-off-by: Hua Yan Cc: dri-devel at lists.freedesktop.org --- include/drm/drm_sarea.h |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/drm/drm_sarea.h b/include/drm/drm_sarea.h index ee5389d..1d1a858 100644 --- a/include/drm/drm_sarea.h +++ b/include/drm/drm_sarea.h @@ -37,6 +37,8 @@ /* SAREA area needs to be at least a page */ #if defined(__alpha__) #define SAREA_MAX 0x2000U +#elif defined(__mips__) +#define SAREA_MAX 0x4000U #elif defined(__ia64__) #define SAREA_MAX 0x1U /* 64kB */ #else -- 1.7.7.3
Linux 3.2: After resume screen only turned on after switching terminals
Am Freitag, den 06.07.2012, 17:39 +0200 schrieb Paul Menzel: > Am Freitag, den 06.07.2012, 17:34 +0200 schrieb Paul Menzel: > > Am Donnerstag, den 05.07.2012, 08:58 -0400 schrieb Alex Deucher: > > > On Wed, Jul 4, 2012 at 10:11 AM, Paul Menzel wrote: > > > > Am Mittwoch, den 04.07.2012, 09:55 -0400 schrieb Alex Deucher: > > > >> On Wed, Jul 4, 2012 at 9:41 AM, Paul Menzel wrote: > > > >> > Am Samstag, den 30.06.2012, 21:00 +0200 schrieb Paul Menzel: > > > >> >> Am Dienstag, den 26.06.2012, 07:42 +0200 schrieb Michel D?nzer: > > > >> >> > On Mon, 2012-06-25 at 21:14 +0200, Paul Menzel wrote: > > > >> >> > > > > > >> >> > > resuming from suspend to RAM the monitor was indicating by a > > > >> >> > > blinking > > > >> >> > > LED that it did not receive any signal. This is the first time > > > >> >> > > this > > > >> >> > > happened. Resuming from suspend to RAM had worked without > > > >> >> > > problems > > > >> >> > > before (and probably will work again the next tries). > > > >> >> > > > > > >> >> > > Logging in from SSH worked fine though. Switching terminal to > > > >> >> > > tty1 > > > >> >> > > nothing changed but going back to tty7, where X was supposed to > > > >> >> > > run), > > > >> >> > > and the monitor detected a signal and showed the screensaver. > > > >> >> > > > > > >> >> > > Switching to tty1 still does not work though, that means the > > > >> >> > > monitor > > > >> >> > > indicates that it gets no signal. > > > >> >> > > > > >> >> > What about tty2-6 or any others? > > > >> >> > > > >> >> It just happened again. tty2?6 also do not show anything. > > > >> >> > > > >> >> > What does fbset -i say when the problem occurs? > > > >> >> > > > >> >> I logged in over SSH not having switched the ttys yet. This is the > > > >> >> result. > > > >> >> > > > >> >> $ fbset -i > > > >> >> > > > >> >> mode "1280x1024" > > > >> >> geometry 1280 1024 1280 1024 32 > > > >> >> timings 0 0 0 0 0 0 0 > > > >> >> accel true > > > >> >> rgba 8/16,8/8,8/0,0/0 > > > >> >> endmode > > > >> >> > > > >> >> Frame buffer device information: > > > >> >> Name: radeondrmfb > > > >> >> Address : 0xd0142000 > > > >> >> Size: 5242880 > > > >> >> Type: PACKED PIXELS > > > >> >> Visual : TRUECOLOR > > > >> >> XPanStep: 1 > > > >> >> YPanStep: 1 > > > >> >> YWrapStep : 0 > > > >> >> LineLength : 5120 > > > >> >> Accelerator : No > > > >> >> > > > >> >> This does not differ though after having switched between ttys and > > > >> >> getting a working X server again. The other ttys do not just show > > > >> >> black. > > > >> >> When switching from tty7 with the X server running to a console(?) > > > >> >> tty > > > >> >> then I shortly (half second) see some kind of test image, which > > > >> >> contains > > > >> >> four horizontal colored stripes red, green, blue and white(?). > > > >> >> > > > >> >> I could not find anything in the output of `dmesg`, but > > > >> >> `/var/log/syslog` contains several of the following lines. > > > >> >> > > > >> >> Jun 29 19:44:57 debian-host acpid: client 8677[0:0] has > > > >> >> disconnected > > > >> >> Jun 29 19:45:08 debian-host acpid: client connected from > > > >> >> 8677[0:0] > > > >> >> Jun 29 19:45:08 debian-host acpid: 1 client rule loaded > > > >> > > > > >> > Does someone have an idea what the problem might be and how to fix > > > >> > it? > > > >> > > > >> Is this a regression? If so can you bisect? > > > > > > > > I do not think so. But the problem is I just got that board, have only > > > > used it with Linux 3.2.x (from Debian Sid/unstable) and this issue is > > > > not reproducible, that means, it happens in less then 5(?) percent of > > > > the suspend and resume cycles. > > > > > > > > Any idea what these acpid messages could mean? > > > > It just happened again. This time with the monitor not getting any > > signal. Switching to other virtual terminals does not work. > > > > What debug levels should I increase? > > I attach the output of `dmesg`. Yesterday it finally happened again. After resume the monitor stayed black. Only switching virtual consoles with Ctrl + Alt + F6 and back fixed this. This is the time for the resume. [ 8688.180746] [drm:drm_helper_probe_single_connector_modes], [CONNECTOR:15:DVI-D-1] disconnected [ 9069.400626] PM: Syncing filesystems ... done. Please find the output of `dmesg` and `drm.debug=0x06` attached. Thanks, Paul -- next part -- [ 8673.624413] calling usb3+ @ 21003, parent: :00:12.0 [ 8673.624417] initcall usb3_i+ returned 0 after 1 usecs [ 8673.624421] calling usb2+ @ 21003, parent: :00:13.2 [ 8673.624426] initcall usb2_i+ returned 0 after 1 usecs [ 8673.624430] calling usb1+ @ 21003, parent: :00:12.2 [
[PATCH 3/3] gma500: Consider CRTC initially active.
From: Forest Bond This causes the pipe to be forced off prior to initial mode set, which roughly mirrors the behavior of the i915 driver. It fixes initial mode setting on my Intel DN2800MT (Cedarview) board. Without it, mode setting triggers an out-of-range error from the monitor for most modes, but only on initial configuration (i.e. they can be configured successfully from userspace after that). Signed-off-by: Forest Bond --- drivers/gpu/drm/gma500/psb_intel_display.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index 30dc22a..8033526 100644 --- a/drivers/gpu/drm/gma500/psb_intel_display.c +++ b/drivers/gpu/drm/gma500/psb_intel_display.c @@ -1362,6 +1362,9 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe, (struct drm_connector **) (psb_intel_crtc + 1); psb_intel_crtc->mode_set.num_connectors = 0; psb_intel_cursor_init(dev, psb_intel_crtc); + + /* Set to true so that the pipe is forced off on initial config. */ + psb_intel_crtc->active = true; } int psb_intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, -- 1.7.0.4
[PATCH 1/3] gma500: Fix comment mispelling in cdv_intel_limits definition.
From: Forest Bond
Signed-off-by: Forest Bond
---
drivers/gpu/drm/gma500/cdv_intel_display.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c
b/drivers/gpu/drm/gma500/cdv_intel_display.c
index a68509b..883a9f3 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -65,7 +65,7 @@ struct cdv_intel_limit_t {
#define CDV_LIMIT_DAC_HDMI_96 3
static const struct cdv_intel_limit_t cdv_intel_limits[] = {
- { /* CDV_SIGNLE_LVDS_96MHz */
+ { /* CDV_SINGLE_LVDS_96MHz */
.dot = {.min = 2, .max = 115500},
.vco = {.min = 180, .max = 360},
.n = {.min = 2, .max = 6},
--
1.7.0.4
[PATCH 2/3] gma: psb_intel_crtc: Drop crtc_enable flag.
From: Forest Bond
This is set when setting DPMS on and off, but it isn't checked anywhere,
so just remove it.
Signed-off-by: Forest Bond
---
drivers/gpu/drm/gma500/cdv_intel_display.c |2 --
drivers/gpu/drm/gma500/psb_intel_drv.h |1 -
2 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c
b/drivers/gpu/drm/gma500/cdv_intel_display.c
index 883a9f3..2100aff 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -841,7 +841,6 @@ static void cdv_intel_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* Give the overlay scaler a chance to enable
* if it's on this pipe */
/* psb_intel_crtc_dpms_video(crtc, true); TODO */
- psb_intel_crtc->crtc_enable = true;
break;
case DRM_MODE_DPMS_OFF:
if (!psb_intel_crtc->active)
@@ -893,7 +892,6 @@ static void cdv_intel_crtc_dpms(struct drm_crtc *crtc, int
mode)
/* Wait for the clocks to turn off. */
udelay(150);
cdv_intel_update_watermark(dev, crtc);
- psb_intel_crtc->crtc_enable = false;
break;
}
/*Set FIFO Watermarks*/
diff --git a/drivers/gpu/drm/gma500/psb_intel_drv.h
b/drivers/gpu/drm/gma500/psb_intel_drv.h
index ebe1a28..e179c36 100644
--- a/drivers/gpu/drm/gma500/psb_intel_drv.h
+++ b/drivers/gpu/drm/gma500/psb_intel_drv.h
@@ -190,7 +190,6 @@ struct psb_intel_crtc {
u32 mode_flags;
bool active;
- bool crtc_enable;
/* Saved Crtc HW states */
struct psb_intel_crtc_state *crtc_state;
--
1.7.0.4
Kconfig: warning: (DRM_RADEON_KMS && DRM_I915 && …) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT)
Dear Linux folks, where should I report the following warning warning: (DRM_RADEON_KMS && DRM_I915 && STUB_POULSBO && FB_BACKLIGHT && USB_APPLEDISPLAY && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10 && APPLE_GMUX) selects BACKLIGHT_CLASS_DEVICE which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) I get with the following commit? commit f4ba394c1b02e7fc2179fda8d3941a5b3b65efb6 Merge: bf44ce8 5d299f3 Author: Linus Torvalds Date: Wed Aug 8 20:06:43 2012 +0300 Thanks, Paul -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/0d469754/attachment.pgp>
[Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
>> A dma-fence can be attached to a buffer which is being filled or consumed
>> by hw, to allow userspace to pass the buffer without waiting to another
>> device. For example, userspace can call page_flip ioctl to display the
>> next frame of graphics after kicking the GPU but while the GPU is still
>> rendering. The display device sharing the buffer with the GPU would
>> attach a callback to get notified when the GPU's rendering-complete IRQ
>> fires, to update the scan-out address of the display, without having to
>> wake up userspace.
>>
>> A dma-fence is transient, one-shot deal. It is allocated and attached
>> to one or more dma-buf's. When the one that attached it is done, with
>> the pending operation, it can signal the fence.
>>
>> + dma_fence_signal()
>>
>> The dma-buf-mgr handles tracking, and waiting on, the fences associated
>> with a dma-buf.
>>
>> TODO maybe need some helper fxn for simple devices, like a display-
>> only drm/kms device which simply wants to wait for exclusive fence to
>> be signaled, and then attach a non-exclusive fence while scanout is in
>> progress.
>>
>> The one pending on the fence can add an async callback:
>> + dma_fence_add_callback()
>> The callback can optionally be cancelled with remove_wait_queue()
>>
>> Or wait synchronously (optionally with timeout or interruptible):
>> + dma_fence_wait()
>>
>> A default software-only implementation is provided, which can be used
>> by drivers attaching a fence to a buffer when they have no other means
>> for hw sync. But a memory backed fence is also envisioned, because it
>> is common that GPU's can write to, or poll on some memory location for
>> synchronization. For example:
>>
>> fence = dma_buf_get_fence(dmabuf);
>> if (fence->ops == &bikeshed_fence_ops) {
>> dma_buf *fence_buf;
>> dma_bikeshed_fence_get_buf(fence, &fence_buf, &offset);
>> ... tell the hw the memory location to wait on ...
>> } else {
>> /* fall-back to sw sync * /
>> dma_fence_add_callback(fence, my_cb);
>> }
>>
>> On SoC platforms, if some other hw mechanism is provided for synchronizing
>> between IP blocks, it could be supported as an alternate implementation
>> with it's own fence ops in a similar way.
>>
>> To facilitate other non-sw implementations, the enable_signaling callback
>> can be used to keep track if a device not supporting hw sync is waiting
>> on the fence, and in this case should arrange to call dma_fence_signal()
>> at some point after the condition has changed, to notify other devices
>> waiting on the fence. If there are no sw waiters, this can be skipped to
>> avoid waking the CPU unnecessarily. The handler of the enable_signaling
>> op should take a refcount until the fence is signaled, then release its ref.
>>
>> The intention is to provide a userspace interface (presumably via eventfd)
>> later, to be used in conjunction with dma-buf's mmap support for sw access
>> to buffers (or for userspace apps that would prefer to do their own
>> synchronization).
>
> I think the commit message should be cleaned up: Kill the TODO, rip out
> the bikeshed_fence and otherwise update it to the latest code.
>
>>
>> v1: Original
>> v2: After discussion w/ danvet and mlankhorst on #dri-devel, we decided
>> that dma-fence didn't need to care about the sw->hw signaling path
>> (it can be handled same as sw->sw case), and therefore the fence->ops
>> can be simplified and more handled in the core. So remove the signal,
>> add_callback, cancel_callback, and wait ops, and replace with a simple
>> enable_signaling() op which can be used to inform a fence supporting
>> hw->hw signaling that one or more devices which do not support hw
>> signaling are waiting (and therefore it should enable an irq or do
>> whatever is necessary in order that the CPU is notified when the
>> fence is passed).
>> v3: Fix locking fail in attach_fence() and get_fence()
>> v4: Remove tie-in w/ dma-buf.. after discussion w/ danvet and mlankorst
>> we decided that we need to be able to attach one fence to N dma-buf's,
>> so using the list_head in dma-fence struct would be problematic.
>> v5: [ Maarten Lankhorst ] Updated for dma-bikeshed-fence and dma-buf-manager.
>> v6: [ Maarten Lankhorst ] I removed dma_fence_cancel_callback and some
>> comments
>> about checking if fence fired or not. This is broken by design.
>> waitqueue_active during destruction is now fatal, since the signaller
>> should be holding a reference in enable_signalling until it signalled
>> the fence. Pass the original dma_fence_cb along, and call __remove_wait
>> in the dma_fence_callback handler, so that no cleanup needs to be
>> performed.
>> v7: [ Maarten Lankhorst ] Set cb->func and only enable sw signaling if
>> fence wasn't signaled yet, for example for hardware fence
[Linaro-mm-sig] [PATCH 1/4] dma-buf: remove fallback for !CONFIG_DMA_SHARED_BUFFER
On Fri, Aug 10, 2012 at 2:32 PM, Daniel Vetter wrote: > On Fri, Aug 10, 2012 at 04:57:43PM +0200, Maarten Lankhorst wrote: >> Documentation says that code requiring dma-buf should add it to >> select, so inline fallbacks are not going to be used. A link error >> will make it obvious what went wrong, instead of silently doing >> nothing at runtime. >> >> Signed-off-by: Maarten Lankhorst > > I've botched it more than once to update these when creating new dma-buf > code. Hence > > Reviewed-by: Daniel Vetter yeah, I think the fallbacks date back to when it was a user configurable option, rather than something select'd by drivers using dmabuf, and we just never went back to clean up. Let's drop the fallbacks. Reviewed-by: Rob Clark > -- > Daniel Vetter > Mail: daniel at ffwll.ch > Mobile: +41 (0)79 365 57 48 > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm: EDID quirk improvements
On 08/11/2012 03:31 AM, Paul Menzel wrote: > As a side note, could you also mention the patch iteration in the tag, > that means [PATCH vN] so that I know what is the latest version. That > would be great. Can you (or anyone else reading this) point me to how to do this with git send-email? > I would submit adding the new quirk flags and the LG quirk as separate > patches. If it is not too much work, it would be great if you could > split them up. Does git provide any facility to make this easier? As far as I can tell, the process is to start over with a newly cloned repository, apply the current patch, manually back out the changes that I want to separate, commit, manually redo the separate changes, and commit again. Is that correct? > You even added this one from the following commit. > > commit bc42aabc6a01b92b0f961d65671564e0e1cd7592 > Author: Adam Jackson > Date: Wed May 23 16:26:54 2012 -0400 > > drm/edid/quirks: ViewSonic VA2026w > > I am going to need that quirk. Great! It's in Linus's tree. >> +DEFINE_MUTEX(edid_quirk_list_mutex); >> + >> +/** >> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for >> printing >> + * @mfg_id: the encoded manufacturer ID >> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 >> bytes) > > format*t*ed > > http://www.merriam-webster.com/dictionary/formatted Thanks for catching that. I keep finding little typos like that; very annoying. >> + >> +return count; >> +} > > Add an empty line here before the next comment? > >> /*** DDC fetch and block validation ***/ I'm pretty sure I can manage that. ;-) >> +MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt"); > > Not all users have access to the Linux source tree, so maybe a small > overview is still needed? Or even an URL? I can't think of a way to provide anything useful within the scope of a parameter description. Any suggestions? A URL would be great, but what would it be? (I don't have a personal web site, and that doesn't seem really appropriate anyway.) > Thanks again for that great patch. With the comments addressed above you > can add my acknowledgment. > > Acked-by: Paul Menzel Thank you for your feedback. Are you saying that I should add the acked-by? If so, how? (You can probably tell that I'm really struggling with git.) > I am going to try to test that patch too for a Philips and LG TV [2]. I hope it helps. -- Ian Pilcher arequipeno at gmail.com "If you're going to shift my paradigm ... at least buy me dinner first."
[Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
Hey,
Op 11-08-12 17:14, Rob Clark schreef:
> On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
>> On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
>>> A dma-fence can be attached to a buffer which is being filled or consumed
>>> by hw, to allow userspace to pass the buffer without waiting to another
>>> device. For example, userspace can call page_flip ioctl to display the
>>> next frame of graphics after kicking the GPU but while the GPU is still
>>> rendering. The display device sharing the buffer with the GPU would
>>> attach a callback to get notified when the GPU's rendering-complete IRQ
>>> fires, to update the scan-out address of the display, without having to
>>> wake up userspace.
>>>
>>> A dma-fence is transient, one-shot deal. It is allocated and attached
>>> to one or more dma-buf's. When the one that attached it is done, with
>>> the pending operation, it can signal the fence.
>>>
>>> + dma_fence_signal()
>>>
>>> The dma-buf-mgr handles tracking, and waiting on, the fences associated
>>> with a dma-buf.
>>>
>>> TODO maybe need some helper fxn for simple devices, like a display-
>>> only drm/kms device which simply wants to wait for exclusive fence to
>>> be signaled, and then attach a non-exclusive fence while scanout is in
>>> progress.
>>>
>>> The one pending on the fence can add an async callback:
>>> + dma_fence_add_callback()
>>> The callback can optionally be cancelled with remove_wait_queue()
>>>
>>> Or wait synchronously (optionally with timeout or interruptible):
>>> + dma_fence_wait()
>>>
>>> A default software-only implementation is provided, which can be used
>>> by drivers attaching a fence to a buffer when they have no other means
>>> for hw sync. But a memory backed fence is also envisioned, because it
>>> is common that GPU's can write to, or poll on some memory location for
>>> synchronization. For example:
>>>
>>> fence = dma_buf_get_fence(dmabuf);
>>> if (fence->ops == &bikeshed_fence_ops) {
>>> dma_buf *fence_buf;
>>> dma_bikeshed_fence_get_buf(fence, &fence_buf, &offset);
>>> ... tell the hw the memory location to wait on ...
>>> } else {
>>> /* fall-back to sw sync * /
>>> dma_fence_add_callback(fence, my_cb);
>>> }
>>>
>>> On SoC platforms, if some other hw mechanism is provided for synchronizing
>>> between IP blocks, it could be supported as an alternate implementation
>>> with it's own fence ops in a similar way.
>>>
>>> To facilitate other non-sw implementations, the enable_signaling callback
>>> can be used to keep track if a device not supporting hw sync is waiting
>>> on the fence, and in this case should arrange to call dma_fence_signal()
>>> at some point after the condition has changed, to notify other devices
>>> waiting on the fence. If there are no sw waiters, this can be skipped to
>>> avoid waking the CPU unnecessarily. The handler of the enable_signaling
>>> op should take a refcount until the fence is signaled, then release its ref.
>>>
>>> The intention is to provide a userspace interface (presumably via eventfd)
>>> later, to be used in conjunction with dma-buf's mmap support for sw access
>>> to buffers (or for userspace apps that would prefer to do their own
>>> synchronization).
>> I think the commit message should be cleaned up: Kill the TODO, rip out
>> the bikeshed_fence and otherwise update it to the latest code.
Agreed.
>>> v1: Original
>>> v2: After discussion w/ danvet and mlankhorst on #dri-devel, we decided
>>> that dma-fence didn't need to care about the sw->hw signaling path
>>> (it can be handled same as sw->sw case), and therefore the fence->ops
>>> can be simplified and more handled in the core. So remove the signal,
>>> add_callback, cancel_callback, and wait ops, and replace with a simple
>>> enable_signaling() op which can be used to inform a fence supporting
>>> hw->hw signaling that one or more devices which do not support hw
>>> signaling are waiting (and therefore it should enable an irq or do
>>> whatever is necessary in order that the CPU is notified when the
>>> fence is passed).
>>> v3: Fix locking fail in attach_fence() and get_fence()
>>> v4: Remove tie-in w/ dma-buf.. after discussion w/ danvet and mlankorst
>>> we decided that we need to be able to attach one fence to N dma-buf's,
>>> so using the list_head in dma-fence struct would be problematic.
>>> v5: [ Maarten Lankhorst ] Updated for dma-bikeshed-fence and
>>> dma-buf-manager.
>>> v6: [ Maarten Lankhorst ] I removed dma_fence_cancel_callback and some
>>> comments
>>> about checking if fence fired or not. This is broken by design.
>>> waitqueue_active during destruction is now fatal, since the signaller
>>> should be holding a reference in enable_signalling until it signalled
>>> the fence. Pass the original dma_fence_cb along, and call __remove_wait
>>> in the dma_fence_callback handler, so that no cleanup needs to be
>>> perform
[Linaro-mm-sig] [PATCH 3/4] dma-seqno-fence: Hardware dma-buf implementation of fencing (v2)
Op 10-08-12 21:57, Daniel Vetter schreef:
> On Fri, Aug 10, 2012 at 04:57:58PM +0200, Maarten Lankhorst wrote:
>> This type of fence can be used with hardware synchronization for simple
>> hardware that can block execution until the condition
>> (dma_buf[offset] - value) >= 0 has been met.
>>
>> A software fallback still has to be provided in case the fence is used
>> with a device that doesn't support this mechanism. It is useful to expose
>> this for graphics cards that have an op to support this.
>>
>> Some cards like i915 can export those, but don't have an option to wait,
>> so they need the software fallback.
>>
>> I extended the original patch by Rob Clark.
>>
>> v1: Original
>> v2: Renamed from bikeshed to seqno, moved into dma-fence.c since
>> not much was left of the file. Lots of documentation added.
>>
>> Signed-off-by: Maarten Lankhorst
> Patch looks good, two bikesheds inline. Either way
> Reviewed-by: Daniel Vetter
>
>> ---
>> drivers/base/dma-fence.c | 21 +++
>> include/linux/dma-fence.h | 61
>> +
>> 2 files changed, 82 insertions(+)
>>
>> diff --git a/drivers/base/dma-fence.c b/drivers/base/dma-fence.c
>> index 93448e4..4092a58 100644
>> --- a/drivers/base/dma-fence.c
>> +++ b/drivers/base/dma-fence.c
>> @@ -266,3 +266,24 @@ struct dma_fence *dma_fence_create(void *priv)
>> return fence;
>> }
>> EXPORT_SYMBOL_GPL(dma_fence_create);
>> +
>> +static int seqno_enable_signaling(struct dma_fence *fence)
>> +{
>> +struct dma_seqno_fence *seqno_fence = to_seqno_fence(fence);
>> +return seqno_fence->enable_signaling(seqno_fence);
>> +}
>> +
>> +static void seqno_release(struct dma_fence *fence)
>> +{
>> +struct dma_seqno_fence *f = to_seqno_fence(fence);
>> +
>> +if (f->release)
>> +f->release(f);
>> +dma_buf_put(f->sync_buf);
>> +}
>> +
>> +const struct dma_fence_ops dma_seqno_fence_ops = {
>> +.enable_signaling = seqno_enable_signaling,
>> +.release = seqno_release
>> +};
>> +EXPORT_SYMBOL_GPL(dma_seqno_fence_ops);
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index e0ceddd..3ef0da0 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -91,6 +91,19 @@ struct dma_fence_ops {
>> void (*release)(struct dma_fence *fence);
>> };
>>
>> +struct dma_seqno_fence {
>> +struct dma_fence base;
>> +
>> +struct dma_buf *sync_buf;
>> +uint32_t seqno_ofs;
>> +uint32_t seqno;
>> +
>> +int (*enable_signaling)(struct dma_seqno_fence *fence);
>> +void (*release)(struct dma_seqno_fence *fence);
> I think using dma_fence_ops here is the better color. We lose type-safety
> at compile-time, but still keep type-safety at runtime (thanks to
> to_dma_seqno_fence). In addition people seem to like to constify function
> pointers, we'd save a pointer and if we extend the sw dma_fence interface.
Ok, will change.
>> +};
>> +
>> +extern const struct dma_fence_ops dma_seqno_fence_ops;
>> +
>> struct dma_fence *dma_fence_create(void *priv);
>>
>> /**
>> @@ -121,4 +134,52 @@ int dma_fence_wait(struct dma_fence *fence, bool intr,
>> unsigned long timeout);
>> int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,
>> dma_fence_func_t func, void *priv);
>>
>> +/**
>> + * to_seqno_fence - cast a dma_fence to a dma_seqno_fence
>> + * @fence: dma_fence to cast to a dma_seqno_fence
>> + *
>> + * Returns NULL if the dma_fence is not a dma_seqno_fence,
>> + * or the dma_seqno_fence otherwise.
>> + */
>> +static inline struct dma_seqno_fence *
>> +to_seqno_fence(struct dma_fence *fence)
>> +{
>> +if (fence->ops != &dma_seqno_fence_ops)
>> +return NULL;
>> +return container_of(fence, struct dma_seqno_fence, base);
>> +}
> I think adding an is_dma_seqno_fence would be nice ...
#define is_dma_seqno_fence !!to_dma_seqno_fence
The first thing you would do after finding out it's a seqno fence
is calling to_dma_seqno_fence, otherwise why would you care?
As such the check was pointless and deleted.
My bikeshed, go build your own!
>> +
>> +/**
>> + * dma_seqno_fence_init - initialize a seqno fence
>> + * @fence: dma_seqno_fence to initialize
>> + * @sync_buf: buffer containing the memory location to signal on
>> + * @seqno_ofs: the offset within @sync_buf
>> + * @seqno: the sequence # to signal on
>> + * @priv: value of priv member
>> + * @enable_signaling: callback which is called when some other device is
>> + *waiting for sw notification of fence
>> + * @release: callback called during destruction before object is freed.
>> + *
>> + * This function initializes a struct dma_seqno_fence with passed
>> parameters,
>> + * and takes a reference on sync_buf which is released on fence destruction.
>> + */
>> +static inline void
>> +dma_seqno_fence_init(struct dma_seqno_fence *fence,
>> +struct dma_buf *sync_buf,
>> +uint32_t seqno
[Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 10:14:40AM -0500, Rob Clark wrote:
> On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> > On Fri, Aug 10, 2012 at 04:57:52PM +0200, Maarten Lankhorst wrote:
> >> +
> >> + if (!ret) {
> >> + cb->base.flags = 0;
> >> + cb->base.func = __dma_fence_wake_func;
> >> + cb->base.private = priv;
> >> + cb->fence = fence;
> >> + cb->func = func;
> >> + __add_wait_queue(&fence->event_queue, &cb->base);
> >> + }
> >> + spin_unlock_irqrestore(&fence->event_queue.lock, flags);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(dma_fence_add_callback);
> >
> > I think for api completenes we should also have a
> > dma_fence_remove_callback function.
>
> We did originally but Maarten found it was difficult to deal with
> properly when the gpu's hang. I think his alternative was just to
> require the hung driver to signal the fence. I had kicked around the
> idea of a dma_fence_cancel() alternative to signal that could pass an
> error thru to the waiting driver.. although not sure if the other
> driver could really do anything differently at that point.
Well, the idea is not to cancel all callbacks, but just a single one, in
case a driver wants to somehow abort the wait. E.g. when the own gpu dies
I guess we should clear all these fence callbacks so that they can't
clobber the hw state after the reset.
> >> +
> >> +/**
> >> + * dma_fence_wait - wait for a fence to be signaled
> >> + *
> >> + * @fence: [in]The fence to wait on
> >> + * @intr:[in]if true, do an interruptible wait
> >> + * @timeout: [in]absolute time for timeout, in jiffies.
> >
> > I don't quite like this, I think we should keep the styl of all other
> > wait_*_timeout functions and pass the arg as timeout in jiffies (and also
> > the same return semantics). Otherwise well have funny code that needs to
> > handle return values differently depending upon whether it waits upon a
> > dma_fence or a native object (where it would us the wait_*_timeout
> > functions directly).
>
> We did start out this way, but there was an ugly jiffies roll-over
> problem that was difficult to deal with properly. Using an absolute
> time avoided the problem.
Well, as-is the api works differently than all the other _timeout apis
I've seen in the kernel, which makes it confusing. Also, I don't quite see
what jiffies wraparound issue there is?
> > Also, I think we should add the non-_timeout variants, too, just for
> > completeness.
This request here has the same reasons, essentially: If we offer a
dma_fence wait api that matches the usual wait apis closely, it's harder
to get their usage wrong. I know that i915 has some major cludge for a
wait_seqno interface internally, but that's no reason to copy that
approach ;-)
Cheers, Daniel
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
[ANNOUNCE] libdrm 2.4.38
Alex Deucher (2): radeon: add some missing evergreen pci ids radeon: add some new SI pci ids Chris Wilson (1): intel: Bail gracefully if we encounter an unknown Intel device Cooper Yuan (1): libdrm/exynos: padding gem_mmap structure to 64-bit aligned Damien Lespiau (1): intel: Remove two unused variables Dave Airlie (4): libdrm: add missing caps from kernel to drm.h libdrm: add prime fd->handle and handle->fd interfaces libdrm/nouveau: add prime handle->bo and bo->handle support. intel: add prime interface for getting/setting a prime bo. (v4) Eric Anholt (4): intel: Quiet valgrind warnings in context creation. Drop "-Wunsafe-loop-optimizations". intel: Import updated i915_drm.h. intel: Add a function for the new register read ioctl. Kenneth Graunke (1): intel: Change context create failure message to from fprintf to DBG(). Laurent Pinchart (8): modetest: Unify buffer allocation modetest: Add SMPTE test pattern modetest: Add test pattern support for missing packed YUV formats modetest: Add test pattern support for missing planar YUV formats modetest: Add test pattern support for missing RGB formats modetest: Move connector and plane parsing to separate functions modetest: Make frame buffer format configurable on the command line modeset: Split buffer allocation to a separate file Lauri Kasanen (1): intel: Fix build failure in test_decode.c Marek Ol??k (6): radeon: simplify ZS buffer checking on r600 radeon: optimize allocation for depth w/o stencil and stencil w/o depth on EG radeon: force 2D tiling for MSAA surfaces radeon: tweak TILE_SPLIT for MSAA surfaces tests/modetest: fix distcheck configure: bump version for 2.4.38 release Paulo Zanoni (1): intel: add more Haswell PCI IDs Rob Clark (5): omap: clarify dmabuf file descriptor ownership omap: add API to import bo's from dmabuf fd's omap: add refcnting and handle tracking intel: fix build error modetest: fix uninitialized fourcc git tag: libdrm-2.4.38 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.38.tar.bz2 MD5: 8018e0bce5059cee05d855503d262cce libdrm-2.4.38.tar.bz2 SHA1: 21718ddb8be71bc74845a33c2b4fbda1de942e16 libdrm-2.4.38.tar.bz2 SHA256: a7caec55c945f7f8dec55fea9200391a168298bcdc4cb9a93c976e748193171a libdrm-2.4.38.tar.bz2 http://dri.freedesktop.org/www/libdrm/libdrm-2.4.38.tar.gz MD5: 9aa553e543d716f2f9b9405274c5d9c3 libdrm-2.4.38.tar.gz SHA1: 8a98f0332b33d647918cc8c4bf41fbe1e21a15cb libdrm-2.4.38.tar.gz SHA256: 75d586a5a989a572e63a8f1252a51dfee4cdeb0146f8b9fe5ecd1b6844e85a20 libdrm-2.4.38.tar.gz
[PATCH] drm: EDID quirk improvements
Dear Ian, Am Freitag, den 10.08.2012, 13:44 -0500 schrieb Ian Pilcher: [?] > diff --git a/Documentation/EDID/edid_quirks.txt > b/Documentation/EDID/edid_quirks.txt > new file mode 100644 > index 000..256ded0 > --- /dev/null > +++ b/Documentation/EDID/edid_quirks.txt [?] > +Overview > + > + > +EDID quirks provide a mechanism for working around display hardware with > buggy > +EDID data. > + > +An individual EDID quirk maps a display type (identified by its EDID > +manufacturer ID and product code[1]) to a set of flags. For example, the > current > +list of quirks built into the kernel is: > + > +ACR:0xad46:0x0001 > +API:0x7602:0x0001 > +ACR:0x0977:0x0020 > +MAX:0x05ec:0x0001 > +MAX:0x077e:0x0001 > +EPI:0xe780:0x0002 > +EPI:0x2028:0x0001 > +FCM:0x3520:0x000c > +LPL:0x:0x0010 > +LPL:0x2a00:0x0010 > +PHL:0xe014:0x0020 > +PTS:0x02fd:0x0020 > +SAM:0x021d:0x0040 > +SAM:0x0254:0x0001 > +SAM:0x027e:0x0001 > +VSC:0x139c:0x0080 > +GSM:0x563f:0x0300 reading the document again, I guess keeping this list up to date will be forgotten and duplicating this information is not necessary. Maybe just add one or two example quirks. I cannot think of a `grep` command to run to list all quirks, so maybe just mention that all the quirks are stored in `edid_quirk_list[]` in `drivers/gpu/drm/drm_edid.c` [1]. > + > +The first field of each quirk is the manufacturer ID, the second field is the > +product code, and the third field is the quirk flags. > + > +NOTE: All of the manufacturer IDs above are displayed as 3-character strings, > +because they are conformant IDs that have been properly encoded: > + > +- The most-significant bit of the encoded ID is 0 > +- They only contain ASCII characters in the range A-Z > + > +IDs that do not conform to these rules are displayed as "raw" hexadecimal > +values. > + > +The current quirk flags are: > + > +/* First detailed mode wrong, use largest 60Hz mode */ > +#define EDID_QUIRK_PREFER_LARGE_60 0x0001 > + > +/* Reported 135MHz pixel clock is too high, needs adjustment */ > +#define EDID_QUIRK_135_CLOCK_TOO_HIGH 0x0002 > + > +/* Prefer the largest mode at 75 Hz */ > +#define EDID_QUIRK_PREFER_LARGE_75 0x0004 > + > +/* Detail timing is in cm not mm */ > +#define EDID_QUIRK_DETAILED_IN_CM 0x0008 > + > +/* Detailed timing descriptors have bogus size values, so just take the > + * maximum size and use that. > + */ > +#define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE0x0010 > + > +/* Monitor forgot to set the first detailed is preferred bit. */ > +#define EDID_QUIRK_FIRST_DETAILED_PREFERRED 0x0020 > + > +/* use +hsync +vsync for detailed mode */ > +#define EDID_QUIRK_DETAILED_SYNC_PP 0x0040 > + > +/* Force reduced-blanking timings for detailed modes */ > +#define EDID_QUIRK_FORCE_REDUCED_BLANKING 0x0080 > + > +/* Display is confused by InfoFrames; don't sent any */ > +#define EDID_QUIRK_DISABLE_INFOFRAMES 0x0100 > + > +/* Display doesn't have any audio output */ > +#define EDID_QUIRK_NO_AUDIO 0x0200 That might be also hard to keep up to date. Maybe also just note that these quirks are defined in the beginning of `drivers/gpu/drm/drm_edid.c` [1] and that these are bit shifts(?) [2]. [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c;h=a8743c399e83234c976ebdb4b471542a0645c42d;hb=HEAD [2] https://en.wikipedia.org/wiki/Bitwise_operation#Shifts_in_C.2C_C.2B.2B.2C_C.23 [?] Thanks, Paul -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/0f054575/attachment.pgp>
[Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 06:00:46PM +0200, Maarten Lankhorst wrote:
> Op 11-08-12 17:14, Rob Clark schreef:
> > On Fri, Aug 10, 2012 at 3:29 PM, Daniel Vetter wrote:
> >>> +/**
> >>> + * dma_fence_signal - signal completion of a fence
> >>> + * @fence: the fence to signal
> >>> + *
> >>> + * All registered callbacks will be called directly (synchronously) and
> >>> + * all blocked waters will be awoken. This should be always be called on
> >>> + * software only fences, or alternatively be called after
> >>> + * dma_fence_ops::enable_signaling is called.
> >> I think we need to be cleare here when dma_fence_signal can be called:
> >> - for a sw-only fence (i.e. created with dma_fence_create)
> >> dma_fence_signal _must_ be called under all circumstances.
> >> - for any other fences, dma_fence_signal may be called, but it _must_ be
> >> called once the ->enable_signalling func has been called and return 0
> >> (i.e. success).
> >> - it may be called only _once_.
>
> As we discussed on irc it might be beneficial to be able to have it called
> twice, the second time would be a noop, however.
Agreed.
[snip]
> >>> + /* At this point, if enable_signaling returns any error
> >>> + * a wakeup has to be performanced regardless.
> >>> + * -ENOENT signals fence was already signaled. Any other
> >>> error
> >>> + * inidicates a catastrophic hardware error.
> >>> + *
> >>> + * If any hardware error occurs, nothing can be done against
> >>> + * it, so it's treated like the fence was already signaled.
> >>> + * No synchronization can be performed, so we have to assume
> >>> + * the fence was already signaled.
> >>> + */
> >>> + ret = fence->ops->enable_signaling(fence);
> >>> + if (ret) {
> >>> + signaled = true;
> >>> + dma_fence_signal(fence);
> >> I think we should call dma_fence_signal only for -ENOENT and pass all
> >> other errors back as-is. E.g. on -ENOMEM or so we might want to retry ...
>
> Also discussed on irc, boolean might be a better solution until we start
> dealing with
> hardware on fire. :) This would however likely be dealt in the same way as
> signaling,
> however.
Agreed.
[snip]
> >>> +
> >>> + if (!ret) {
> >>> + cb->base.flags = 0;
> >>> + cb->base.func = __dma_fence_wake_func;
> >>> + cb->base.private = priv;
> >>> + cb->fence = fence;
> >>> + cb->func = func;
> >>> + __add_wait_queue(&fence->event_queue, &cb->base);
> >>> + }
> >>> + spin_unlock_irqrestore(&fence->event_queue.lock, flags);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(dma_fence_add_callback);
> >> I think for api completenes we should also have a
> >> dma_fence_remove_callback function.
> > We did originally but Maarten found it was difficult to deal with
> > properly when the gpu's hang. I think his alternative was just to
> > require the hung driver to signal the fence. I had kicked around the
> > idea of a dma_fence_cancel() alternative to signal that could pass an
> > error thru to the waiting driver.. although not sure if the other
> > driver could really do anything differently at that point.
>
> No, there is a very real reason I removed dma_fence_remove_callback. It is
> absolutely non-trivial to cancel it once added, since you have to deal with
> all kinds of race conditions.. See i915_gem_reset_requests in my git tree:
> http://cgit.freedesktop.org/~mlankhorst/linux/commit/?id=673c4b2550bc63ec134bca47a95dabd617a689ce
I don't see the point in that code ... Why can't we drop the kref
_outside_ of the critical section protected by event_queue_lock? Then you
pretty much have an open-coded version of dma_fence_callback_cancel in
there.
> This is the only way to do it completely deadlock/memory corruption free
> since you essentially have a locking inversion to avoid. I had it wrong
> the first 2 times too, even when I knew about a lot of the locking
> complications. If you want to do it, in most cases it will likely be easier
> to just eat the signal and ignore it instead of canceling.
>
> >>> +
> >>> +/**
> >>> + * dma_fence_wait - wait for a fence to be signaled
> >>> + *
> >>> + * @fence: [in]The fence to wait on
> >>> + * @intr:[in]if true, do an interruptible wait
> >>> + * @timeout: [in]absolute time for timeout, in jiffies.
> >> I don't quite like this, I think we should keep the styl of all other
> >> wait_*_timeout functions and pass the arg as timeout in jiffies (and also
> >> the same return semantics). Otherwise well have funny code that needs to
> >> handle return values differently depending upon whether it waits upon a
> >> dma_fence or a native object (where it would us the wait_*_timeout
> >> functions directly).
> > We did start out this way, but there was an ugly jiffies roll
[PATCH] drm: EDID quirk improvements
Dear Ian, Am Samstag, den 11.08.2012, 10:38 -0500 schrieb Ian Pilcher: > On 08/11/2012 03:31 AM, Paul Menzel wrote: > > As a side note, could you also mention the patch iteration in the tag, > > that means [PATCH vN] so that I know what is the latest version. That > > would be great. > > Can you (or anyone else reading this) point me to how to do this with > git send-email? No, but you can do that with `git format-patch` [3]. git format-patch --subject-prefix="PATCH v6" ? > > I would submit adding the new quirk flags and the LG quirk as separate > > patches. If it is not too much work, it would be great if you could > > split them up. > > Does git provide any facility to make this easier? As far as I can > tell, the process is to start over with a newly cloned repository, > apply the current patch, manually back out the changes that I want to > separate, commit, manually redo the separate changes, and commit > again. Is that correct? Kind of. But you can do that in the same clone within your checkout/clone. Basically you go before your commit and then use `git add -p` to add only the hunks to the commit you need [4][5]. Do not worry that your changes get lost. `git reflog` still shows you the commit hash of your original patch. You can always go back to that by doing `git reset --hard ` or checking it out in another branch `git checkout -b myoldpatch_v5 `. You can read the manual with `git help commandname` like `git help add`. The folks in #git on irc.freenode.net are also very helpful. [?] > >> +DEFINE_MUTEX(edid_quirk_list_mutex); > >> + > >> +/** > >> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for > >> printing > >> + * @mfg_id: the encoded manufacturer ID > >> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 > >> bytes) > > > > format*t*ed > > > > http://www.merriam-webster.com/dictionary/formatted > > Thanks for catching that. I keep finding little typos like that; very > annoying. Well, it was the only typo I found. I thought you are using a spell checker already. [?] > I can't think of a way to provide anything useful within the scope of a > parameter description. Any suggestions? Maybe just say give an example `vendor:model:quirk`. > A URL would be great, but what would it be? (I don't have a personal > web site, and that doesn't seem really appropriate anyway.) http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c > > Thanks again for that great patch. With the comments addressed above you > > can add my acknowledgment. > > > > Acked-by: Paul Menzel > > Thank you for your feedback. > > Are you saying that I should add the acked-by? If so, how? (You can > probably tell that I'm really struggling with git.) That is very easy: `git commit --amend` to just edit the commit message. You can edit the other changes above, by editing the appropriate files and then doing `git commit --amend -a`. This adds all changes in files known to Git also to the last commit. > > I am going to try to test that patch too for a Philips and LG TV [2]. > > I hope it helps. We will see tomorrow. Thanks, Paul [3] http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotating_new_revision [4] http://stackoverflow.com/questions/4307095/git-how-to-split-up-a-commit-buried-in-history [5] http://git.661346.n2.nabble.com/How-to-split-a-big-commit-td6238260.html -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: This is a digitally signed message part URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/e694b7c4/attachment-0001.pgp>
ping: Re: quirk for Samsung 2443BW
Hello Adam, On Fri, Jul 20, 2012 at 12:34:21PM +1000, Dave Airlie wrote: > > Samsung 2443BW is 1920x1200 but reports 1920x1080 in the EDID. Attached > > is a proof-of-concept implementation of a quirk. It works on my i686 PC. ... > > This implementation matches the wrong mode by size. Other approaches are > > possible. > > > > I'd appreciate feedback. > > ajax, seem sane? Ping? With kind regards, Baurzhan. -- next part -- A non-text attachment was scrubbed... Name: 0001-drm-Make-edid_quirk_list-const.patch Type: text/x-diff Size: 1294 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/61baa9dd/attachment.patch> -- next part -- A non-text attachment was scrubbed... Name: 0002-drm-Add-quirk-for-Samsung-SyncMaster-2443BW.patch Type: text/x-diff Size: 6886 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/61baa9dd/attachment-0001.patch>
[Linaro-mm-sig] [PATCH 2/4] dma-fence: dma-buf synchronization (v8 )
On Sat, Aug 11, 2012 at 2:22 PM, Daniel Vetter wrote: >> >> + >> >> +/** >> >> + * dma_fence_wait - wait for a fence to be signaled >> >> + * >> >> + * @fence: [in]The fence to wait on >> >> + * @intr:[in]if true, do an interruptible wait >> >> + * @timeout: [in]absolute time for timeout, in jiffies. >> > >> > I don't quite like this, I think we should keep the styl of all other >> > wait_*_timeout functions and pass the arg as timeout in jiffies (and also >> > the same return semantics). Otherwise well have funny code that needs to >> > handle return values differently depending upon whether it waits upon a >> > dma_fence or a native object (where it would us the wait_*_timeout >> > functions directly). >> >> We did start out this way, but there was an ugly jiffies roll-over >> problem that was difficult to deal with properly. Using an absolute >> time avoided the problem. > > Well, as-is the api works differently than all the other _timeout apis > I've seen in the kernel, which makes it confusing. Also, I don't quite see > what jiffies wraparound issue there is? iirc, the problem was in dmabufmgr, in dmabufmgr_wait_completed_cpu().. with an absolute timeout, it could loop over all the fences without having to adjust the timeout for the elapsed time. Otherwise it had to adjust the timeout and keep track of when the timeout elapsed without confusing itself via rollover. BR, -R
[PATCH v3 0/2] Enhanced EDID quirk functionality
Updated patch set, based on Paul's feedback. * Separate user-defined quirks stuff from new HDMI-related quirks * (Hopefully) improve documentation Also continuing to explore the wonders of git format-patch and git send-email. Ian Pilcher (2): drm: Add user-defined EDID quirks capability drm: Add EDID quirks to disable HDMI audio and InfoFrames Documentation/EDID/edid_quirks.txt | 126 + drivers/gpu/drm/drm_drv.c | 2 + drivers/gpu/drm/drm_edid.c | 524 + drivers/gpu/drm/drm_stub.c | 6 + drivers/gpu/drm/drm_sysfs.c| 19 ++ include/drm/drmP.h | 10 + include/drm/drm_edid.h | 13 +- 7 files changed, 639 insertions(+), 61 deletions(-) create mode 100644 Documentation/EDID/edid_quirks.txt -- 1.7.11.2
[PATCH v3 1/2] drm: Add user-defined EDID quirks capability
Add the ability for users to define their own EDID quirks via a
module parameter or sysfs attribute.
Signed-off-by: Ian Pilcher
Acked-by: Paul Menzel
---
Documentation/EDID/edid_quirks.txt | 126 ++
drivers/gpu/drm/drm_drv.c | 2 +
drivers/gpu/drm/drm_edid.c | 500 -
drivers/gpu/drm/drm_stub.c | 6 +
drivers/gpu/drm/drm_sysfs.c| 19 ++
include/drm/drmP.h | 10 +
include/drm/drm_edid.h | 13 +-
7 files changed, 615 insertions(+), 61 deletions(-)
create mode 100644 Documentation/EDID/edid_quirks.txt
diff --git a/Documentation/EDID/edid_quirks.txt
b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 000..0c9c746
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,126 @@
+ EDID Quirks
+ =
+ Ian Pilcher
+August 11, 2012
+
+
+"EDID blocks out in the wild have a variety of bugs"
+-- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of "quirk flags." The kernel
+includes a variety of built-in quirks. (They are stored in the edid_quirk_list
+array in drivers/gpu/drm/drm_edid.c.)
+
+An example of a built-in EDID quirk is:
+
+ACR:0xad46:0x0001
+
+The first field is the manufacturer ID (Acer, Inc.), the second field is the
+manufacturer's product code, and the third field contains the quirk flags for
+that display type.
+
+The quirk flags are defined in drivers/gpu/drm/drm_edid.c. Each flag has a
+symbolic name beginning with EDID_QUIRK_, along with a numerical value. Each
+flag should also have an associated comment which provides an idea of its
+effect. Note that the values in the source file are expressed as bit
shifts[2]:
+
+* 1 << 0: 0x0001
+* 1 << 1: 0x0002
+* 1 << 2: 0x0004
+* etc.
+
+
+sysfs interface
+===
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks:
+
+# cat /sys/class/drm/edid_quirks
+ ACR:0xad46:0x0001
+ API:0x7602:0x0001
+ ACR:0x0977:0x0020
+0x9e6a:0x077e:0x0080
+...
+
+("Nonconformant" manufacturer IDs are displayed as hexadecimal values.)
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size. This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks. The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to /sys/class/drm/edid_quirks:
+
+# echo FOO:0x:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically. (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+# echo 0x19ef:0x:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+# echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+# echo FOO:0x:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the slot occupied by that quirk available.
+
+# echo FOO:0x:0 > /sys/class/drm/edid_quirks
+
+Writing an "at symbol" (@) clears the entire quirk list:
+
+# echo @ > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+# echo @,FOO:0x:0x100,BAR:0x:0x001,BAZ:0x:0x002 > \
+/sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made. For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line). The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference. When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the paramet
[PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames
Add EDID quirk flags to disable HDMI audio and HDMI InfoFrames.
Add quirk for LG L246WP.
Signed-off-by: Ian Pilcher
Acked-by: Paul Menzel
---
drivers/gpu/drm/drm_edid.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bb3ba20..6c143ed 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -70,6 +70,10 @@
#define EDID_QUIRK_DETAILED_SYNC_PP(1 << 6)
/* Force reduced-blanking timings for detailed modes */
#define EDID_QUIRK_FORCE_REDUCED_BLANKING (1 << 7)
+/* Display is confused by InfoFrames; don't sent any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES (1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO(1 << 9)
struct detailed_mode_closure {
struct drm_connector *connector;
@@ -156,6 +160,10 @@ union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+ /* LG L246WP */
+ { { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+ EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
/*
* When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
* provide some room for user-supplied quirks.
@@ -2109,6 +2117,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
int i, hdmi_id;
int start_offset, end_offset;
bool is_hdmi = false;
+ char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+ if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+ DRM_INFO("Disabling HDMI InfoFrames on display %s "
+"due to EDID quirk\n",
+drm_edid_display_id_format(edid->display_id, buf, 1));
+ goto end;
+ }
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
@@ -2157,6 +2173,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
int i, j;
bool has_audio = false;
int start_offset, end_offset;
+ char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+ if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+ DRM_INFO("Disabling HDMI audio on display %s "
+"due to EDID quirk\n",
+drm_edid_display_id_format(edid->display_id, buf, 1));
+ goto end;
+ }
edid_ext = drm_find_cea_extension(edid);
if (!edid_ext)
--
1.7.11.2
[BUG] EDID leaks kernel memory
Hi, While looking at the kernel DRM code, I've noticed that in many places we kmalloc() memory to store the raw EDID information, whether it be from a DDC adapter, or loaded from firmware. Nowhere can I find where this memory is freed. It seems in several places that we assign it into connector->display_info.raw_edid, and next time we obtain EDID information, we re-kmalloc and overwrite this pointer. Note that some drivers do kfree() this memory themselves after blindly setting connector->display_info.raw_edid to NULL... Can someone please point me to where this memory is freed? If not, I'll cook up a patch to add some kfree's. Thanks.
[Patch v2 1/4] Replace i2f() in r600_blit.c with an optimized version.
We use __fls() to find the most significant bit. Using that, the
loop can be avoided. A second trick is to use the behaviour of the
rotate instructions to expand the range of the unsigned int to float
conversion to the full 32 bits in a branchless way.
The routine is now exact up to 2^24. Above that, we truncate which
is equivalent to rounding towards zero.
Signed-off-by: Steven Fuerst
---
drivers/gpu/drm/radeon/r600_blit.c | 50
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600_blit.c
b/drivers/gpu/drm/radeon/r600_blit.c
index 3c031a4..326a8da 100644
--- a/drivers/gpu/drm/radeon/r600_blit.c
+++ b/drivers/gpu/drm/radeon/r600_blit.c
@@ -489,29 +489,35 @@ set_default_state(drm_radeon_private_t *dev_priv)
ADVANCE_RING();
}
-static uint32_t i2f(uint32_t input)
+/* 23 bits of float fractional data */
+#define I2F_FRAC_BITS 23
+#define I2F_MASK ((1 << I2F_FRAC_BITS) - 1)
+
+/*
+ * Converts unsigned integer into 32-bit IEEE floating point representation.
+ * Will be exact from 0 to 2^24. Above that, we round towards zero
+ * as the fractional bits will not fit in a float. (It would be better to
+ * round towards even as the fpu does, but that is slower.)
+ */
+static uint32_t i2f(uint32_t x)
{
- u32 result, i, exponent, fraction;
-
- if ((input & 0x3fff) == 0)
- result = 0; /* 0 is a special case */
- else {
- exponent = 140; /* exponent biased by 127; */
- fraction = (input & 0x3fff) << 10; /* cheat and only
- handle numbers below
2^^15 */
- for (i = 0; i < 14; i++) {
- if (fraction & 0x80)
- break;
- else {
- fraction = fraction << 1; /* keep
-shifting left
until top bit = 1 */
- exponent = exponent - 1;
- }
- }
- result = exponent << 23 | (fraction & 0x7f); /* mask
- off top
bit; assumed 1 */
- }
- return result;
+ uint32_t msb, exponent, fraction;
+
+ /* Zero is special */
+ if (!x) return 0;
+
+ /* Get location of the most significant bit */
+ msb = __fls(x);
+
+ /*
+* Use a rotate instead of a shift because that works both leftwards
+* and rightwards due to the mod(32) behaviour. This means we don't
+* need to check to see if we are above 2^24 or not.
+*/
+ fraction = ror32(x, (msb - I2F_FRAC_BITS) & 0x1f) & I2F_MASK;
+ exponent = (127 + msb) << I2F_FRAC_BITS;
+
+ return fraction + exponent;
}
--
1.7.10.4
[Patch v2 2/4] Replace i2f() in r600_blit_kms.c with an optimized version.
We use __fls() to find the most significant bit. Using that, the
loop can be avoided. A second trick is to use the behaviour of the
rotate instructions to expand the range of the unsigned int to float
conversion to the full 32 bits in a branchless way.
The routine is now exact up to 2^24. Above that, we truncate which
is equivalent to rounding towards zero.
Signed-off-by: Steven Fuerst
---
drivers/gpu/drm/radeon/r600_blit_kms.c | 51 +---
1 file changed, 21 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c
b/drivers/gpu/drm/radeon/r600_blit_kms.c
index 2bef854..e5a40ca 100644
--- a/drivers/gpu/drm/radeon/r600_blit_kms.c
+++ b/drivers/gpu/drm/radeon/r600_blit_kms.c
@@ -455,44 +455,35 @@ set_default_state(struct radeon_device *rdev)
radeon_ring_write(ring, sq_stack_resource_mgmt_2);
}
-#define I2F_MAX_BITS 15
-#define I2F_MAX_INPUT ((1 << I2F_MAX_BITS) - 1)
-#define I2F_SHIFT (24 - I2F_MAX_BITS)
+/* 23 bits of float fractional data */
+#define I2F_FRAC_BITS 23
+#define I2F_MASK ((1 << I2F_FRAC_BITS) - 1)
/*
* Converts unsigned integer into 32-bit IEEE floating point representation.
- * Conversion is not universal and only works for the range from 0
- * to 2^I2F_MAX_BITS-1. Currently we only use it with inputs between
- * 0 and 16384 (inclusive), so I2F_MAX_BITS=15 is enough. If necessary,
- * I2F_MAX_BITS can be increased, but that will add to the loop iterations
- * and slow us down. Conversion is done by shifting the input and counting
- * down until the first 1 reaches bit position 23. The resulting counter
- * and the shifted input are, respectively, the exponent and the fraction.
- * The sign is always zero.
+ * Will be exact from 0 to 2^24. Above that, we round towards zero
+ * as the fractional bits will not fit in a float. (It would be better to
+ * round towards even as the fpu does, but that is slower.)
*/
-static uint32_t i2f(uint32_t input)
+static uint32_t i2f(uint32_t x)
{
- u32 result, i, exponent, fraction;
+ uint32_t msb, exponent, fraction;
- WARN_ON_ONCE(input > I2F_MAX_INPUT);
+ /* Zero is special */
+ if (!x) return 0;
- if ((input & I2F_MAX_INPUT) == 0)
- result = 0;
- else {
- exponent = 126 + I2F_MAX_BITS;
- fraction = (input & I2F_MAX_INPUT) << I2F_SHIFT;
+ /* Get location of the most significant bit */
+ msb = __fls(x);
- for (i = 0; i < I2F_MAX_BITS; i++) {
- if (fraction & 0x80)
- break;
- else {
- fraction = fraction << 1;
- exponent = exponent - 1;
- }
- }
- result = exponent << 23 | (fraction & 0x7f);
- }
- return result;
+ /*
+* Use a rotate instead of a shift because that works both leftwards
+* and rightwards due to the mod(32) behaviour. This means we don't
+* need to check to see if we are above 2^24 or not.
+*/
+ fraction = ror32(x, (msb - I2F_FRAC_BITS) & 0x1f) & I2F_MASK;
+ exponent = (127 + msb) << I2F_FRAC_BITS;
+
+ return fraction + exponent;
}
int r600_blit_init(struct radeon_device *rdev)
--
1.7.10.4
[Patch v2 3/4] Rename i2f() to int2float(), and make it global so one copy can be removed.
Remove the copy of i2f() in r600_blit_kms.c
We rename the function to something longer now that it is a global
symbol. This reduces the likelyhood of unintended clashes later.
This might be a candidate for inclusion inside general drm infrastructure.
However, at the moment only the radeon driver uses it.
Signed-off-by: Steven Fuerst
Conflicts:
drivers/gpu/drm/radeon/r600_blit_kms.c
---
drivers/gpu/drm/radeon/r600_blit.c | 67 ++--
drivers/gpu/drm/radeon/r600_blit_kms.c | 43 +++---
drivers/gpu/drm/radeon/r600_blit_shaders.h |1 +
3 files changed, 40 insertions(+), 71 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600_blit.c
b/drivers/gpu/drm/radeon/r600_blit.c
index 326a8da..54980d8 100644
--- a/drivers/gpu/drm/radeon/r600_blit.c
+++ b/drivers/gpu/drm/radeon/r600_blit.c
@@ -499,7 +499,7 @@ set_default_state(drm_radeon_private_t *dev_priv)
* as the fractional bits will not fit in a float. (It would be better to
* round towards even as the fpu does, but that is slower.)
*/
-static uint32_t i2f(uint32_t x)
+uint32_t int2float(uint32_t x)
{
uint32_t msb, exponent, fraction;
@@ -520,7 +520,6 @@ static uint32_t i2f(uint32_t x)
return fraction + exponent;
}
-
static int r600_nomm_get_vb(struct drm_device *dev)
{
drm_radeon_private_t *dev_priv = dev->dev_private;
@@ -638,20 +637,20 @@ r600_blit_copy(struct drm_device *dev,
vb = r600_nomm_get_vb_ptr(dev);
}
- vb[0] = i2f(dst_x);
+ vb[0] = int2float(dst_x);
vb[1] = 0;
- vb[2] = i2f(src_x);
+ vb[2] = int2float(src_x);
vb[3] = 0;
- vb[4] = i2f(dst_x);
- vb[5] = i2f(h);
- vb[6] = i2f(src_x);
- vb[7] = i2f(h);
+ vb[4] = int2float(dst_x);
+ vb[5] = int2float(h);
+ vb[6] = int2float(src_x);
+ vb[7] = int2float(h);
- vb[8] = i2f(dst_x + cur_size);
- vb[9] = i2f(h);
- vb[10] = i2f(src_x + cur_size);
- vb[11] = i2f(h);
+ vb[8] = int2float(dst_x + cur_size);
+ vb[9] = int2float(h);
+ vb[10] = int2float(src_x + cur_size);
+ vb[11] = int2float(h);
/* src */
set_tex_resource(dev_priv, FMT_8,
@@ -727,20 +726,20 @@ r600_blit_copy(struct drm_device *dev,
vb = r600_nomm_get_vb_ptr(dev);
}
- vb[0] = i2f(dst_x / 4);
+ vb[0] = int2float(dst_x / 4);
vb[1] = 0;
- vb[2] = i2f(src_x / 4);
+ vb[2] = int2float(src_x / 4);
vb[3] = 0;
- vb[4] = i2f(dst_x / 4);
- vb[5] = i2f(h);
- vb[6] = i2f(src_x / 4);
- vb[7] = i2f(h);
+ vb[4] = int2float(dst_x / 4);
+ vb[5] = int2float(h);
+ vb[6] = int2float(src_x / 4);
+ vb[7] = int2float(h);
- vb[8] = i2f((dst_x + cur_size) / 4);
- vb[9] = i2f(h);
- vb[10] = i2f((src_x + cur_size) / 4);
- vb[11] = i2f(h);
+ vb[8] = int2float((dst_x + cur_size) / 4);
+ vb[9] = int2float(h);
+ vb[10] = int2float((src_x + cur_size) / 4);
+ vb[11] = int2float(h);
/* src */
set_tex_resource(dev_priv, FMT_8_8_8_8,
@@ -810,20 +809,20 @@ r600_blit_swap(struct drm_device *dev,
dx2 = dx + w;
dy2 = dy + h;
- vb[0] = i2f(dx);
- vb[1] = i2f(dy);
- vb[2] = i2f(sx);
- vb[3] = i2f(sy);
+ vb[0] = int2float(dx);
+ vb[1] = int2float(dy);
+ vb[2] = int2float(sx);
+ vb[3] = int2float(sy);
- vb[4] = i2f(dx);
- vb[5] = i2f(dy2);
- vb[6] = i2f(sx);
- vb[7] = i2f(sy2);
+ vb[4] = int2float(dx);
+ vb[5] = int2float(dy2);
+ vb[6] = int2float(sx);
+ vb[7] = int2float(sy2);
- vb[8] = i2f(dx2);
- vb[9] = i2f(dy2);
- vb[10] = i2f(sx2);
- vb[11] = i2f(sy2);
+ vb[8] = int2float(dx2);
+ vb[9] = int2float(dy2);
+ vb[10] = int2float(sx2);
+ vb[11] = int2float(sy2);
switch(cpp) {
case 4:
diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c
b/drivers/gpu/drm/radeon/r600_blit_kms.c
index e5a40ca..1c7ed3a 100644
--- a/drivers/gpu/drm/radeon/r600_blit_kms.c
[Patch v2 4/4] Annotate int2float() as being a pure function.
This allows gcc to fold duplicate calls into a single call. Since
the current users do actually call it multiple times with the
same arguments, this is an obvious win.
---
drivers/gpu/drm/radeon/r600_blit.c |2 +-
drivers/gpu/drm/radeon/r600_blit_shaders.h |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/r600_blit.c
b/drivers/gpu/drm/radeon/r600_blit.c
index 54980d8..b4304288 100644
--- a/drivers/gpu/drm/radeon/r600_blit.c
+++ b/drivers/gpu/drm/radeon/r600_blit.c
@@ -499,7 +499,7 @@ set_default_state(drm_radeon_private_t *dev_priv)
* as the fractional bits will not fit in a float. (It would be better to
* round towards even as the fpu does, but that is slower.)
*/
-uint32_t int2float(uint32_t x)
+__pure uint32_t int2float(uint32_t x)
{
uint32_t msb, exponent, fraction;
diff --git a/drivers/gpu/drm/radeon/r600_blit_shaders.h
b/drivers/gpu/drm/radeon/r600_blit_shaders.h
index e17c2cb..2f3ce7a 100644
--- a/drivers/gpu/drm/radeon/r600_blit_shaders.h
+++ b/drivers/gpu/drm/radeon/r600_blit_shaders.h
@@ -35,5 +35,5 @@ extern const u32 r6xx_default_state[];
extern const u32 r6xx_ps_size, r6xx_vs_size;
extern const u32 r6xx_default_size, r7xx_default_size;
-uint32_t int2float(uint32_t x);
+__pure uint32_t int2float(uint32_t x);
#endif
--
1.7.10.4
[PATCH] Fix hw_i2c option for RV280 and RV350.
Hi, I've been carrying this fix privately for far too long; without it, I cannot enable the hardware I2C for either of my Radeons RV280 or RV350. It does not fix the I2C for my rv100, which remains broken. The nature of the failure is that the display fails completely: [ 35.067825] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 230 [ 35.076023] Raw EDID: [ 35.078315] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff [ 35.084067] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 35.089820] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ 35.095574] ff ff ff ff ff ff ff ff ff 00 00 ff ff ff ff ff [ 35.101328] ff ff 1e 1e ff ff ff ff ff ff ff 6d 6d ff ff ff [ 35.107080] ff ff ff ff 3e 3e ff ff ff ff ff ff ff 4e 4e ff [ 35.112833] ff ff ff ff ff ff 9a 9a ff ff ff ff ff ff ff f7 [ 35.118587] f7 ff ff ff ff ff ff ff ff ff ff 02 02 ff ff ff [ 35.167737] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 142 Signed-off-by: Chris Rankin Cheers, Chris -- next part -- A non-text attachment was scrubbed... Name: RADEON_I2C.diff Type: text/x-patch Size: 398 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/380ef43a/attachment-0001.bin>
