[PATCH 4/4] ARM: tegra: roth: add display DT node
On Mon, Jul 21, 2014 at 11:16:32PM +0200, Thierry Reding wrote: > On Mon, Jul 21, 2014 at 09:35:27AM -0600, Stephen Warren wrote: > > > vdd-2v8-display needs to remain always-on however. Here we may hit a > > > limitation of the simple-panel driver, where only one power supply can > > > be provided. > > Can't we fix the simple-panel driver to allow a list of regulators in > > the property? > I have no objection to allowing that. But I don't think there's a way to > do that with the current regulator API. You can use the bulk API, but > that requires separate properties, not multiple regulators in one > property. > Perhaps one idea to solve this would be to make the regulator API return > a regulator handle that in fact controls an array of regulators? Adding > Mark. I'm really not comfortable with that idea, it seems like most of the users would be abusing it - one of the biggest issues is always getting people to understand that their driver may be used in other systems with the supplies mapped differently. If you were going to do something along those lines you'd need to do something that enumerated all the supply properties on the device. -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/95478fb9/attachment.sig>
[PATCH v2 00/25] AMDKFD kernel driver
>-Original Message- >From: dri-devel [mailto:dri-devel-bounces at lists.freedesktop.org] On Behalf >Of Jerome Glisse >Sent: Monday, July 21, 2014 7:06 PM >To: Gabbay, Oded >Cc: Lewycky, Andrew; Pinchuk, Evgeny; Daenzer, Michel; linux- >kernel at vger.kernel.org; dri-devel at lists.freedesktop.org; linux-mm; >Skidanov, Alexey; Andrew Morton >Subject: Re: [PATCH v2 00/25] AMDKFD kernel driver > >On Tue, Jul 22, 2014 at 12:56:13AM +0300, Oded Gabbay wrote: >> On 21/07/14 22:28, Jerome Glisse wrote: >> > On Mon, Jul 21, 2014 at 10:23:43PM +0300, Oded Gabbay wrote: >> >> On 21/07/14 21:59, Jerome Glisse wrote: >> >>> On Mon, Jul 21, 2014 at 09:36:44PM +0300, Oded Gabbay wrote: >> On 21/07/14 21:14, Jerome Glisse wrote: >> > On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: >> >> On 21/07/14 18:54, Jerome Glisse wrote: >> >>> On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: >> On 21/07/14 16:39, Christian K?nig wrote: >> > Am 21.07.2014 14:36, schrieb Oded Gabbay: >> >> On 20/07/14 20:46, Jerome Glisse wrote: >> >>> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >> Forgot to cc mailing list on cover letter. Sorry. >> >> As a continuation to the existing discussion, here is a >> v2 patch series restructured with a cleaner history and >> no totally-different-early-versions of the code. >> >> Instead of 83 patches, there are now a total of 25 >> patches, where 5 of them are modifications to radeon driver >and 18 of them include only amdkfd code. >> There is no code going away or even modified between >patches, only added. >> >> The driver was renamed from radeon_kfd to amdkfd and >> moved to reside under drm/radeon/amdkfd. This move was >> done to emphasize the fact that this driver is an >> AMD-only driver at this point. Having said that, we do >> foresee a generic hsa framework being implemented in the >future and in that case, we will adjust amdkfd to work within that >framework. >> >> As the amdkfd driver should support multiple AMD gfx >> drivers, we want to keep it as a seperate driver from >> radeon. Therefore, the amdkfd code is contained in its >> own folder. The amdkfd folder was put under the radeon >> folder because the only AMD gfx driver in the Linux >> kernel at this point is the radeon driver. Having said >> that, we will probably need to move it (maybe to be directly >under drm) after we integrate with additional AMD gfx drivers. >> >> For people who like to review using git, the v2 patch set is >located at: >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-nex >> t-3.17-v2 >> >> Written by Oded Gabbayh >> >>> >> >>> So quick comments before i finish going over all patches. >> >>> There is many things that need more documentation >> >>> espacialy as of right now there is no userspace i can go look at. >> >> So quick comments on some of your questions but first of >> >> all, thanks for the time you dedicated to review the code. >> >>> >> >>> There few show stopper, biggest one is gpu memory pinning >> >>> this is a big no, that would need serious arguments for >> >>> any hope of convincing me on that side. >> >> We only do gpu memory pinning for kernel objects. There are >> >> no userspace objects that are pinned on the gpu memory in >> >> our driver. If that is the case, is it still a show stopper ? >> >> >> >> The kernel objects are: >> >> - pipelines (4 per device) >> >> - mqd per hiq (only 1 per device) >> >> - mqd per userspace queue. On KV, we support up to 1K >> >> queues per process, for a total of 512K queues. Each mqd is >> >> 151 bytes, but the allocation is done in >> >> 256 alignment. So total *possible* memory is 128MB >> >> - kernel queue (only 1 per device) >> >> - fence address for kernel queue >> >> - runlists for the CP (1 or 2 per device) >> > >> > The main questions here are if it's avoid able to pin down >> > the memory and if the memory is pinned down at driver load, >> > by request from userspace or by anything else. >> > >> > As far as I can see only the "mqd per userspace queue" might >> > be a bit questionable, everything else sounds reasonable. >> > >> > Christian. >> >> Most of the pin downs are done on device initialization. >> The "mqd per userspace" is done per userspace queue creation. >> However, as
[Bug 81476] three monitors on two radeon cards works with some layouts not others
https://bugs.freedesktop.org/show_bug.cgi?id=81476 --- Comment #19 from Ian! D. Allen --- (In reply to comment #18) > if all three of your monitors are identical use the same modeline and > pixel clock, you'd only need one PLL so you should be fine. Yes: Three identical 1600x1200. I just have to find an available card that can handle three DVI connectors, ideally without having to buy an active adapter. (Sapphire makes a "Flex" card that claims to do this; my initial attempts to find one to buy have turned up nothing useful.) > TBH, I think it's a hw limitation on r5xx hardware that's not properly > handled. I'd believe it as a hardware limitation if inverting the first two monitors didn't fix it. If I were willing to physically turn my first two monitors upside-down, it would work fine with the given hardware. The hardware can display across my three monitors at 4800x1200 as long as two of the screens are inverted. We just need to make the software put those screens into the hardware the other way up! Thanks for your comments. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/248c0fd3/attachment.html>
[PATCH 4/4] ARM: tegra: roth: add display DT node
On Mon, Jul 21, 2014 at 09:35:27AM -0600, Stephen Warren wrote:
> On 07/02/2014 09:10 PM, Alexandre Courbot wrote:
> > On 07/03/2014 12:55 AM, Stephen Warren wrote:
> >> On 07/02/2014 06:19 AM, Alexandre Courbot wrote:
> >>> DSI support has been fixed to support continuous clock behavior that the
> >>> panel used on SHIELD requires, so finally add its device tree node since
> >>> it is functional.
> >>
> >>> diff --git a/arch/arm/boot/dts/tegra114-roth.dts
> >>> b/arch/arm/boot/dts/tegra114-roth.dts
> >>
> >>> +host1x at 5000 {
> >>> +dsi at 5430 {
> >>
> >> That node looks fine, but I wonder why we need to mark a bunch of
> >> regulators as always-on? shouldn't the references to vdd-supply and
> >> power-supply from this node be enough? If not, perhaps the tree
> >> structure of the regulators isn't correct, or the DSI or panel bindings
> >> don't allow enough regulators to be referenced?
> >
> > regulator-always-on is indeed not needed for vdd_lcd. I actually had a
> > patch in my tree removing this line that I forgot to squash. Will post a
> > v2 for this patch that fixes this, thanks.
> >
> > vdd-2v8-display needs to remain always-on however. Here we may hit a
> > limitation of the simple-panel driver, where only one power supply can
> > be provided.
>
> Can't we fix the simple-panel driver to allow a list of regulators in
> the property?
I have no objection to allowing that. But I don't think there's a way to
do that with the current regulator API. You can use the bulk API, but
that requires separate properties, not multiple regulators in one
property.
Perhaps one idea to solve this would be to make the regulator API return
a regulator handle that in fact controls an array of regulators? Adding
Mark.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/6c3166be/attachment.sig>
[Bug 79980] Random radeonsi crashes
https://bugs.freedesktop.org/show_bug.cgi?id=79980 --- Comment #51 from Andy Furniss --- (In reply to comment #42) > Created attachment 102992 [details] [review] > Possible fix v3. > > Updated and largely simplified patch. > > I'm running the third piglit test with it now and so far the system seems to > be stable. Been running (not piglit) for a few days now without crashing. I see it and a couple more fixes are now in agd5f drm-fixes-3.16-wip, so will try that. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/5257ff2a/attachment.html>
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 21:59, Jerome Glisse wrote: > On Mon, Jul 21, 2014 at 09:36:44PM +0300, Oded Gabbay wrote: >> On 21/07/14 21:14, Jerome Glisse wrote: >>> On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: On 21/07/14 18:54, Jerome Glisse wrote: > On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: >> On 21/07/14 16:39, Christian K?nig wrote: >>> Am 21.07.2014 14:36, schrieb Oded Gabbay: On 20/07/14 20:46, Jerome Glisse wrote: > On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >> Forgot to cc mailing list on cover letter. Sorry. >> >> As a continuation to the existing discussion, here is a v2 patch >> series >> restructured with a cleaner history and no >> totally-different-early-versions >> of the code. >> >> Instead of 83 patches, there are now a total of 25 patches, where 5 >> of them >> are modifications to radeon driver and 18 of them include only >> amdkfd code. >> There is no code going away or even modified between patches, only >> added. >> >> The driver was renamed from radeon_kfd to amdkfd and moved to reside >> under >> drm/radeon/amdkfd. This move was done to emphasize the fact that >> this driver >> is an AMD-only driver at this point. Having said that, we do foresee >> a >> generic hsa framework being implemented in the future and in that >> case, we >> will adjust amdkfd to work within that framework. >> >> As the amdkfd driver should support multiple AMD gfx drivers, we >> want to >> keep it as a seperate driver from radeon. Therefore, the amdkfd code >> is >> contained in its own folder. The amdkfd folder was put under the >> radeon >> folder because the only AMD gfx driver in the Linux kernel at this >> point >> is the radeon driver. Having said that, we will probably need to >> move it >> (maybe to be directly under drm) after we integrate with additional >> AMD gfx >> drivers. >> >> For people who like to review using git, the v2 patch set is located >> at: >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >> >> Written by Oded Gabbayh > > So quick comments before i finish going over all patches. There is > many > things that need more documentation espacialy as of right now there is > no userspace i can go look at. So quick comments on some of your questions but first of all, thanks for the time you dedicated to review the code. > > There few show stopper, biggest one is gpu memory pinning this is a > big > no, that would need serious arguments for any hope of convincing me on > that side. We only do gpu memory pinning for kernel objects. There are no userspace objects that are pinned on the gpu memory in our driver. If that is the case, is it still a show stopper ? The kernel objects are: - pipelines (4 per device) - mqd per hiq (only 1 per device) - mqd per userspace queue. On KV, we support up to 1K queues per process, for a total of 512K queues. Each mqd is 151 bytes, but the allocation is done in 256 alignment. So total *possible* memory is 128MB - kernel queue (only 1 per device) - fence address for kernel queue - runlists for the CP (1 or 2 per device) >>> >>> The main questions here are if it's avoid able to pin down the memory >>> and if the >>> memory is pinned down at driver load, by request from userspace or by >>> anything >>> else. >>> >>> As far as I can see only the "mqd per userspace queue" might be a bit >>> questionable, everything else sounds reasonable. >>> >>> Christian. >> >> Most of the pin downs are done on device initialization. >> The "mqd per userspace" is done per userspace queue creation. However, >> as I >> said, it has an upper limit of 128MB on KV, and considering the 2G local >> memory, I think it is OK. >> The runlists are also done on userspace queue creation/deletion, but we >> only >> have 1 or 2 runlists per device, so it is not that bad. > > 2G local memory ? You can not assume anything on userside configuration > some > one might build an hsa computer with 512M and still expect a functioning > desktop. First of all, I'm only considering Kaveri computer, not "hsa" computer. Second, I would imagine we can build some protection around it, like checking total local memory and limit number of queues based on some percentage
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 21:22, Daniel Vetter wrote: > On Mon, Jul 21, 2014 at 7:28 PM, Oded Gabbay wrote: >>> I'm not sure whether we can do the same trick with the hw scheduler. But >>> then unpinning hw contexts will drain the pipeline anyway, so I guess we >>> can just stop feeding the hw scheduler until it runs dry. And then unpin >>> and evict. >> So, I'm afraid but we can't do this for AMD Kaveri because: > > Well as long as you can drain the hw scheduler queue (and you can do > that, worst case you have to unmap all the doorbells and other stuff > to intercept further submission from userspace) you can evict stuff. I can't drain the hw scheduler queue, as I can't do mid-wave preemption. Moreover, if I use the dequeue request register to preempt a queue during a dispatch it may be that some waves (wave groups actually) of the dispatch have not yet been created, and when I reactivate the mqd, they should be created but are not. However, this works fine if you use the HIQ. the CP ucode correctly saves and restores the state of an outstanding dispatch. I don't think we have access to the state from software at all, so it's not a bug, it is "as designed". > And if we don't want compute to be a denial of service on the display > side of the driver we need this ability. Now if you go through an > ioctl instead of the doorbell (I agree with Jerome here, the doorbell > should be supported by benchmarks on linux) this gets a bit easier, > but it's not a requirement really. > -Daniel > On KV, we have the theoretical option of DOS on the display side as we can't do a mid-wave preemption. On CZ, we won't have this problem. Oded
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 21:14, Jerome Glisse wrote: > On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: >> On 21/07/14 18:54, Jerome Glisse wrote: >>> On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: On 21/07/14 16:39, Christian K?nig wrote: > Am 21.07.2014 14:36, schrieb Oded Gabbay: >> On 20/07/14 20:46, Jerome Glisse wrote: >>> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: Forgot to cc mailing list on cover letter. Sorry. As a continuation to the existing discussion, here is a v2 patch series restructured with a cleaner history and no totally-different-early-versions of the code. Instead of 83 patches, there are now a total of 25 patches, where 5 of them are modifications to radeon driver and 18 of them include only amdkfd code. There is no code going away or even modified between patches, only added. The driver was renamed from radeon_kfd to amdkfd and moved to reside under drm/radeon/amdkfd. This move was done to emphasize the fact that this driver is an AMD-only driver at this point. Having said that, we do foresee a generic hsa framework being implemented in the future and in that case, we will adjust amdkfd to work within that framework. As the amdkfd driver should support multiple AMD gfx drivers, we want to keep it as a seperate driver from radeon. Therefore, the amdkfd code is contained in its own folder. The amdkfd folder was put under the radeon folder because the only AMD gfx driver in the Linux kernel at this point is the radeon driver. Having said that, we will probably need to move it (maybe to be directly under drm) after we integrate with additional AMD gfx drivers. For people who like to review using git, the v2 patch set is located at: http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 Written by Oded Gabbayh >>> >>> So quick comments before i finish going over all patches. There is many >>> things that need more documentation espacialy as of right now there is >>> no userspace i can go look at. >> So quick comments on some of your questions but first of all, thanks for >> the >> time you dedicated to review the code. >>> >>> There few show stopper, biggest one is gpu memory pinning this is a big >>> no, that would need serious arguments for any hope of convincing me on >>> that side. >> We only do gpu memory pinning for kernel objects. There are no userspace >> objects that are pinned on the gpu memory in our driver. If that is the >> case, >> is it still a show stopper ? >> >> The kernel objects are: >> - pipelines (4 per device) >> - mqd per hiq (only 1 per device) >> - mqd per userspace queue. On KV, we support up to 1K queues per >> process, for >> a total of 512K queues. Each mqd is 151 bytes, but the allocation is >> done in >> 256 alignment. So total *possible* memory is 128MB >> - kernel queue (only 1 per device) >> - fence address for kernel queue >> - runlists for the CP (1 or 2 per device) > > The main questions here are if it's avoid able to pin down the memory and > if the > memory is pinned down at driver load, by request from userspace or by > anything > else. > > As far as I can see only the "mqd per userspace queue" might be a bit > questionable, everything else sounds reasonable. > > Christian. Most of the pin downs are done on device initialization. The "mqd per userspace" is done per userspace queue creation. However, as I said, it has an upper limit of 128MB on KV, and considering the 2G local memory, I think it is OK. The runlists are also done on userspace queue creation/deletion, but we only have 1 or 2 runlists per device, so it is not that bad. >>> >>> 2G local memory ? You can not assume anything on userside configuration some >>> one might build an hsa computer with 512M and still expect a functioning >>> desktop. >> First of all, I'm only considering Kaveri computer, not "hsa" computer. >> Second, I would imagine we can build some protection around it, like >> checking total local memory and limit number of queues based on some >> percentage of that total local memory. So, if someone will have only >> 512M, he will be able to open less queues. >> >> >>> >>> I need to go look into what all this mqd is for, what it does and what it is >>> about. But pinning is really bad and this is an issue with userspace command >>> scheduling an issue that obviously AMD fails to take into account in design >>> phase. >> Maybe, but that is the H/W design
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 18:54, Jerome Glisse wrote: > On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: >> On 21/07/14 16:39, Christian K?nig wrote: >>> Am 21.07.2014 14:36, schrieb Oded Gabbay: On 20/07/14 20:46, Jerome Glisse wrote: > On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >> Forgot to cc mailing list on cover letter. Sorry. >> >> As a continuation to the existing discussion, here is a v2 patch series >> restructured with a cleaner history and no >> totally-different-early-versions >> of the code. >> >> Instead of 83 patches, there are now a total of 25 patches, where 5 of >> them >> are modifications to radeon driver and 18 of them include only amdkfd >> code. >> There is no code going away or even modified between patches, only added. >> >> The driver was renamed from radeon_kfd to amdkfd and moved to reside >> under >> drm/radeon/amdkfd. This move was done to emphasize the fact that this >> driver >> is an AMD-only driver at this point. Having said that, we do foresee a >> generic hsa framework being implemented in the future and in that case, >> we >> will adjust amdkfd to work within that framework. >> >> As the amdkfd driver should support multiple AMD gfx drivers, we want to >> keep it as a seperate driver from radeon. Therefore, the amdkfd code is >> contained in its own folder. The amdkfd folder was put under the radeon >> folder because the only AMD gfx driver in the Linux kernel at this point >> is the radeon driver. Having said that, we will probably need to move it >> (maybe to be directly under drm) after we integrate with additional AMD >> gfx >> drivers. >> >> For people who like to review using git, the v2 patch set is located at: >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >> >> Written by Oded Gabbayh > > So quick comments before i finish going over all patches. There is many > things that need more documentation espacialy as of right now there is > no userspace i can go look at. So quick comments on some of your questions but first of all, thanks for the time you dedicated to review the code. > > There few show stopper, biggest one is gpu memory pinning this is a big > no, that would need serious arguments for any hope of convincing me on > that side. We only do gpu memory pinning for kernel objects. There are no userspace objects that are pinned on the gpu memory in our driver. If that is the case, is it still a show stopper ? The kernel objects are: - pipelines (4 per device) - mqd per hiq (only 1 per device) - mqd per userspace queue. On KV, we support up to 1K queues per process, for a total of 512K queues. Each mqd is 151 bytes, but the allocation is done in 256 alignment. So total *possible* memory is 128MB - kernel queue (only 1 per device) - fence address for kernel queue - runlists for the CP (1 or 2 per device) >>> >>> The main questions here are if it's avoid able to pin down the memory and >>> if the >>> memory is pinned down at driver load, by request from userspace or by >>> anything >>> else. >>> >>> As far as I can see only the "mqd per userspace queue" might be a bit >>> questionable, everything else sounds reasonable. >>> >>> Christian. >> >> Most of the pin downs are done on device initialization. >> The "mqd per userspace" is done per userspace queue creation. However, as I >> said, it has an upper limit of 128MB on KV, and considering the 2G local >> memory, I think it is OK. >> The runlists are also done on userspace queue creation/deletion, but we only >> have 1 or 2 runlists per device, so it is not that bad. > > 2G local memory ? You can not assume anything on userside configuration some > one might build an hsa computer with 512M and still expect a functioning > desktop. First of all, I'm only considering Kaveri computer, not "hsa" computer. Second, I would imagine we can build some protection around it, like checking total local memory and limit number of queues based on some percentage of that total local memory. So, if someone will have only 512M, he will be able to open less queues. > > I need to go look into what all this mqd is for, what it does and what it is > about. But pinning is really bad and this is an issue with userspace command > scheduling an issue that obviously AMD fails to take into account in design > phase. Maybe, but that is the H/W design non-the-less. We can't very well change the H/W. Oded > >> Oded >>> > > It might be better to add a drivers/gpu/drm/amd directory and add common > stuff there. > > Given that this is not intended to be final HSA api AFAICT then i would > say this far better to avoid the whole kfd module and add ioctl to radeon. > This would a
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Boris, On Monday 21 July 2014 16:18:10 Boris BREZILLON wrote: > On Mon, 21 Jul 2014 15:47:52 +0200 Laurent Pinchart wrote: > > On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > >> On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > >>> On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > >> On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > >>> On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > [snip] > > > The new drm_display_info structure should look like this [2] > > (except that color_formats and bpc have not be removed yet), and > > [1] is just here to show how the video_bus_format enum would > > look like. > > > > [1] http://code.bulix.org/rfd0yx-86557 > > [2] http://code.bulix.org/7n03b4-86556 > > Quoting from your paste: > + const enum video_bus_format *bus_formats; > + int nbus_formats; > > Do we really need more than one? > >>> > >>> We do if we want to replace the color_formats and bpc fields. > >> > >> Yes, that's what I was about to answer :-). > > > > Maybe we don't need to replace color_formats and bpc field > > immediately. That could be done in a follow-up patch. > > We don't need to replace them right now, but we should at least agree > on how to replace them. Introducing a new field that would need to be > replaced in the near future when removing color_formats and bpc would > be a waste of time. > >>> > >>> Sure. One of the problems I see with replacing color_formats and bpc > >>> with the above is that some of the bits within color_formats are set > >>> when the EDID is parsed. That implies that if they are replaced with > >>> an array of formats, the array would need to be reallocated during > >>> EDID parsing. That sounds like ugliness. > >>> > >>> But if you can find a nice way to make it work that'd be great. > >> > >> How about using a list instead of an array ? > >> This way we can add elements to this list when parsing the EDID. > >> > >> Or we can just define a maximum size for the bus_formats array when > >> retrieving this info from EDID. If I'm correct we have at most 18 bus > >> > >> formats: > >> - 3 color formats: > >>* RGB 4:4:4 > >>* YCbCr 4:4:4 > >>* YCbCr 4:4:2 > >> > >> - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color > > > > bpc isn't a bitmask, so EDID supports up to three formats only. > > Yes, bpc only contains a single value for now, and it fits the > DRM_EDID_DIGITAL_DEPTH field [1] (an enum defining the supported pixel > depth). > ITOH, DRM_EDID_HDMI_DC_XX [2] (which are referenced in the new > drm_assign_hdmi_deep_color_info function) are just bitmasks and thus a > display might support several color depth. > > As a result, I wonder if we shouldn't start supporting several color depths > (as we do for color formats) If there's a use case for that, sure. It wouldn't be difficult, given that a bus format defines both the color format and the depths. > [1]http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_edid.c#L3436 > [2]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/driv > ers/gpu/drm/drm_edid.c?id=refs/tags/v3.16-rc6#n3440 -- Regards, Laurent Pinchart
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Boris and Thierry, On Monday 21 July 2014 16:21:36 Boris BREZILLON wrote: > On Mon, 21 Jul 2014 15:54:12 +0200 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 03:47:52PM +0200, Laurent Pinchart wrote: > >> On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > >>> On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > > On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > >> On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > >>> On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: [snip] > > Quoting from your paste: > > + const enum video_bus_format *bus_formats; > > + int nbus_formats; > > > > Do we really need more than one? > > We do if we want to replace the color_formats and bpc fields. > >>> > >>> Yes, that's what I was about to answer :-). > >> > >> Maybe we don't need to replace color_formats and bpc field > >> immediately. That could be done in a follow-up patch. > > > > We don't need to replace them right now, but we should at least > > agree on how to replace them. Introducing a new field that would > > need to be replaced in the near future when removing color_formats > > and bpc would be a waste of time. > > Sure. One of the problems I see with replacing color_formats and bpc > with the above is that some of the bits within color_formats are set > when the EDID is parsed. That implies that if they are replaced with > an array of formats, the array would need to be reallocated during > EDID parsing. That sounds like ugliness. > > But if you can find a nice way to make it work that'd be great. > >>> > >>> How about using a list instead of an array ? > >>> This way we can add elements to this list when parsing the EDID. > >>> > >>> Or we can just define a maximum size for the bus_formats array when > >>> retrieving this info from EDID. If I'm correct we have at most 18 bus > >>> > >>> formats: > >>> - 3 color formats: > >>>* RGB 4:4:4 > >>>* YCbCr 4:4:4 > >>>* YCbCr 4:4:2 > >>> - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color > >> > >> bpc isn't a bitmask, so EDID supports up to three formats only. > >> > >> The color_formats field is computed in the drm_add_display_info() > >> function. You could easily turn it into a local variable and allocate > >> and fill the formats array at the end of the function. > > > > But you also need to be careful to keep whatever formats the driver might > > have set explicitly. Do we have drivers that explicitly add formats to the formats parsed from EDID data ? If so, what's the use case ? > Okay, in this case, using a list is a better idea, don't you think ? I'd prefer an array if possible, as that would be easier to use for drivers. In any case, we need to define who allocates and frees the array or the list elements, how and when. -- Regards, Laurent Pinchart
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 20:05, Daniel Vetter wrote: > On Mon, Jul 21, 2014 at 11:58:52AM -0400, Jerome Glisse wrote: >> On Mon, Jul 21, 2014 at 05:25:11PM +0200, Daniel Vetter wrote: >>> On Mon, Jul 21, 2014 at 03:39:09PM +0200, Christian K?nig wrote: Am 21.07.2014 14:36, schrieb Oded Gabbay: > On 20/07/14 20:46, Jerome Glisse wrote: >> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >>> Forgot to cc mailing list on cover letter. Sorry. >>> >>> As a continuation to the existing discussion, here is a v2 patch series >>> restructured with a cleaner history and no >>> totally-different-early-versions >>> of the code. >>> >>> Instead of 83 patches, there are now a total of 25 patches, where 5 of >>> them >>> are modifications to radeon driver and 18 of them include only amdkfd >>> code. >>> There is no code going away or even modified between patches, only >>> added. >>> >>> The driver was renamed from radeon_kfd to amdkfd and moved to reside >>> under >>> drm/radeon/amdkfd. This move was done to emphasize the fact that this >>> driver >>> is an AMD-only driver at this point. Having said that, we do foresee a >>> generic hsa framework being implemented in the future and in that >>> case, we >>> will adjust amdkfd to work within that framework. >>> >>> As the amdkfd driver should support multiple AMD gfx drivers, we want >>> to >>> keep it as a seperate driver from radeon. Therefore, the amdkfd code is >>> contained in its own folder. The amdkfd folder was put under the radeon >>> folder because the only AMD gfx driver in the Linux kernel at this >>> point >>> is the radeon driver. Having said that, we will probably need to move >>> it >>> (maybe to be directly under drm) after we integrate with additional >>> AMD gfx >>> drivers. >>> >>> For people who like to review using git, the v2 patch set is located >>> at: >>> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >>> >>> Written by Oded Gabbayh >> >> So quick comments before i finish going over all patches. There is many >> things that need more documentation espacialy as of right now there is >> no userspace i can go look at. > So quick comments on some of your questions but first of all, thanks for > the time you dedicated to review the code. >> >> There few show stopper, biggest one is gpu memory pinning this is a big >> no, that would need serious arguments for any hope of convincing me on >> that side. > We only do gpu memory pinning for kernel objects. There are no userspace > objects that are pinned on the gpu memory in our driver. If that is the > case, is it still a show stopper ? > > The kernel objects are: > - pipelines (4 per device) > - mqd per hiq (only 1 per device) > - mqd per userspace queue. On KV, we support up to 1K queues per process, > for a total of 512K queues. Each mqd is 151 bytes, but the allocation is > done in 256 alignment. So total *possible* memory is 128MB > - kernel queue (only 1 per device) > - fence address for kernel queue > - runlists for the CP (1 or 2 per device) The main questions here are if it's avoid able to pin down the memory and if the memory is pinned down at driver load, by request from userspace or by anything else. As far as I can see only the "mqd per userspace queue" might be a bit questionable, everything else sounds reasonable. >>> >>> Aside, i915 perspective again (i.e. how we solved this): When scheduling >>> away from contexts we unpin them and put them into the lru. And in the >>> shrinker we have a last-ditch callback to switch to a default context >>> (since you can't ever have no context once you've started) which means we >>> can evict any context object if it's getting in the way. >> >> So Intel hardware report through some interrupt or some channel when it is >> not using a context ? ie kernel side get notification when some user context >> is done executing ? > > Yes, as long as we do the scheduling with the cpu we get interrupts for > context switches. The mechanic is already published in the execlist > patches currently floating around. We get a special context switch > interrupt. > > But we have this unpin logic already on the current code where we switch > contexts through in-line cs commands from the kernel. There we obviously > use the normal batch completion events. > >> The issue with radeon hardware AFAICT is that the hardware do not report any >> thing about the userspace context running ie you do not get notification when >> a context is not use. Well AFAICT. Maybe hardware do provide that. > > I'm not sure whether we can do the same trick with the hw scheduler. But > then unpinning hw contexts will drain the pipeline anyway, so I guess we > can just stop fee
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 7:28 PM, Oded Gabbay wrote: >> I'm not sure whether we can do the same trick with the hw scheduler. But >> then unpinning hw contexts will drain the pipeline anyway, so I guess we >> can just stop feeding the hw scheduler until it runs dry. And then unpin >> and evict. > So, I'm afraid but we can't do this for AMD Kaveri because: Well as long as you can drain the hw scheduler queue (and you can do that, worst case you have to unmap all the doorbells and other stuff to intercept further submission from userspace) you can evict stuff. And if we don't want compute to be a denial of service on the display side of the driver we need this ability. Now if you go through an ioctl instead of the doorbell (I agree with Jerome here, the doorbell should be supported by benchmarks on linux) this gets a bit easier, but it's not a requirement really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP driver
Hi Thierry,
On Mon, Jul 21, 2014 at 6:24 PM, Thierry Reding
wrote:
> On Mon, Jul 21, 2014 at 04:58:25PM +0530, Ajay kumar wrote:
>> On Mon, Jul 21, 2014 at 12:40 PM, Thierry Reding
>> wrote:
>> > On Fri, Jul 18, 2014 at 02:13:58AM +0530, Ajay Kumar wrote:
>> >> From: Rahul Sharma
>> >>
>> >> This patch adds ps8622 lvds bridge discovery code to the dp driver.
>> >>
>> >> Signed-off-by: Rahul Sharma
>> >> Signed-off-by: Ajay Kumar
>> >> ---
>> >> drivers/gpu/drm/exynos/exynos_dp_core.c |5 +
>> >> 1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> >> b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> >> index 0ca6256..82e2942 100644
>> >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> >> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> >> @@ -31,6 +31,7 @@
>> >> #include
>> >> #include
>> >> #include
>> >> +#include
>> >>
>> >> #include "exynos_drm_drv.h"
>> >> #include "exynos_dp_core.h"
>> >> @@ -999,6 +1000,10 @@ static int exynos_drm_attach_lcd_bridge(struct
>> >> exynos_dp_device *dp,
>> >> if (find_bridge("nxp,ptn3460", &bridge)) {
>> >> bridge_chain = ptn3460_init(dp->drm_dev, encoder,
>> >> bridge.client,
>> >>
>> >> bridge.node);
>> >> + } else if (find_bridge("parade,ps8622", &bridge) ||
>> >> + find_bridge("parade,ps8625", &bridge)) {
>> >> + bridge_chain = ps8622_init(dp->drm_dev, encoder,
>> >> bridge.client,
>> >> +
>> >> bridge.node);
>> >> }
>> >
>> > We really ought to be adding some sort of registry at some point.
>> > Otherwise every driver that wants to use bridges needs to come up with a
>> > similar set of helpers to instantiate them.
>> >
>> > Also you're making this driver depend on (now) two bridges, whereas it
>> > really shouldn't matter which exact types it supports. Bridges should be
>> > exposed via a generic interface.
>>
>> How about moving out the find_bridge() function into a common header file,
>> and also supporting the list of bridge_init declarations in the same file?
>>
>> We get bridge chip node from phandle, and then pass the same node
>> to find_bridge() which searches the list using of_device_is_compatible()
>> to call the appropriate bridge_init function?
>
> That could work, but it's still somewhat unusual and shouldn't be
> required. I think we'd be better of with some sort of registry like we
> have for panels. That would mean that a driver that wants to use a
> bridge would do something like this:
>
> struct drm_bridge *bridge;
> struct device_node *np;
>
> np = of_parse_phandle(dev->of_node, "bridge", 0);
> if (np) {
> bridge = of_drm_find_bridge(np);
> of_node_put(np);
>
> if (!bridge)
> return -EPROBE_DEFER;
> }
>
> An alternative way would be to add a non-OF wrapper around this, like
> this for example:
Let me try the DT version first :)
> bridge = drm_bridge_get(dev, NULL);
>
> Which would be conceptually the same as clk_get() or regulator_get() and
> could be easily extended to support non-DT setups as well.
>
> As for bridge drivers I think we may have to rework things a little, so
> that a driver can call some sequence like this:
>
> struct foo_bridge {
> struct drm_bridge base;
> ...
> };
>
> static const struct drm_bridge_funcs foo_bridge_funcs = {
> ...
> };
>
> static int foo_probe(...)
> {
> struct foo_bridge *bridge;
> int err;
>
> bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
> if (!bridge)
> return -ENOMEM;
>
> /* setup bridge (return -EPROBE_DEFER if necessary, ...) */
>
> /* register bridge with DRM */
> drm_bridge_init(&bridge->base);
> bridge->base.dev = dev;
> bridge->base.funcs = &foo_bridge_funcs;
>
> err = drm_bridge_add(&bridge->base);
> if (err < 0)
> return err;
>
> dev_set_drvdata(dev, bridge);
> ...
> }
>
> drm_bridge_add() would add the bridge to a global list of bridge devices
> which drm_bridge_get()/of_drm_find_bridge() can use to find the one that
> it needs. The above has the big advantage that
> it'sdev->mode_config.bridge_list completely
> independent of the underlying bus that the bridge is on. It could be I2C
> or SPI, platform device or PCI device.
>
> Thierry
Ok. This is all about making the bridge driver confine to the driver model.
But, I would need a drm_device pointer to register the bridge to DRM core.
How do I get it?
Ajay
[Bug 81620] New: radeon: fence wait failed (-35) after hybrid suspend on 3.15
d5ac5ec5cb291, 92858c476ec4e99cf0425f05dee109b6a55eb6f8 and 9e5e7910df824ba02aedd2b5d2ca556426ea6d0b, 76569faa62c46382e080c3e190c66e19515aae1c, de377b3972729f00ee236ae4a97393e282ffe391, 28b6fd6e37792b16a56d324841bdb20ab78e4522, a59ffb2062df3a5c346dbed931fa1e587fd0f0f3 doesn't affect the bug either, so I assume that this bug is not related to suspend/resume async changes. If you cannot reproduce the problem, please advice me what commits to revert. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/bb7bc160/attachment.html>
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 11:29:23PM +, Bridgman, John wrote: > >> >> So even if I really wanted to, and I may agree with you > >> >> theoretically on that, I can't fulfill your desire to make the > >> >> "kernel being able to preempt at any time and be able to decrease > >> >> or increase user queue priority so overall kernel is in charge of > >> >> resources management and it can handle rogue client in proper > >> >> fashion". Not in KV, and I guess not in CZ as well. ^^ > > > >Also it is a worrisome prospect of seeing resource management completely > >ignore for future AMD hardware. Kernel exist for a reason ! Kernel main > >purpose is to provide resource management if AMD fails to understand that, > >this is not looking good on long term and i expect none of the HSA > >technology will get momentum and i would certainly advocate against any > >use of it inside product i work on. > > Hi Jerome; > > I was following along until the above comment. It seems to be the exact > opposite of what Oded has been saying, which is that future AMD hardware > *does* have more capabilities for resource management and that we do have > some capabilities today. Can you help me understand what the comment it was > based on ? Highlighted above. Cheers, J?r?me
[Bug 71812] VDPAU: MPEG-4 ASP Garbling/Corruption
https://bugs.freedesktop.org/show_bug.cgi?id=71812 --- Comment #14 from Adam Hirst --- Created attachment 103214 --> https://bugs.freedesktop.org/attachment.cgi?id=103214&action=edit Corrpution of C into ? at 22.5s For the sake of completion, I can confirm the same corruption of a C into something like an ? at around 22.5s, when playing the file with hardware decoding (`mpv --vo=opengl --hwdec=vdpau --hwdec-codecs=all`) on my AMD PALM unit with VDPAU. Current versions: mesa 10.2.3 xf86-video-ati 7.4.0 linux-ck 3.15.6 xorg-server 1.15.2 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/88a3bb07/attachment.html>
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 11:58:52AM -0400, Jerome Glisse wrote: > On Mon, Jul 21, 2014 at 05:25:11PM +0200, Daniel Vetter wrote: > > On Mon, Jul 21, 2014 at 03:39:09PM +0200, Christian K?nig wrote: > > > Am 21.07.2014 14:36, schrieb Oded Gabbay: > > > >On 20/07/14 20:46, Jerome Glisse wrote: > > > >>On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > > > >>>Forgot to cc mailing list on cover letter. Sorry. > > > >>> > > > >>>As a continuation to the existing discussion, here is a v2 patch series > > > >>>restructured with a cleaner history and no > > > >>>totally-different-early-versions > > > >>>of the code. > > > >>> > > > >>>Instead of 83 patches, there are now a total of 25 patches, where 5 of > > > >>>them > > > >>>are modifications to radeon driver and 18 of them include only amdkfd > > > >>>code. > > > >>>There is no code going away or even modified between patches, only > > > >>>added. > > > >>> > > > >>>The driver was renamed from radeon_kfd to amdkfd and moved to reside > > > >>>under > > > >>>drm/radeon/amdkfd. This move was done to emphasize the fact that this > > > >>>driver > > > >>>is an AMD-only driver at this point. Having said that, we do foresee a > > > >>>generic hsa framework being implemented in the future and in that > > > >>>case, we > > > >>>will adjust amdkfd to work within that framework. > > > >>> > > > >>>As the amdkfd driver should support multiple AMD gfx drivers, we want > > > >>>to > > > >>>keep it as a seperate driver from radeon. Therefore, the amdkfd code is > > > >>>contained in its own folder. The amdkfd folder was put under the radeon > > > >>>folder because the only AMD gfx driver in the Linux kernel at this > > > >>>point > > > >>>is the radeon driver. Having said that, we will probably need to move > > > >>>it > > > >>>(maybe to be directly under drm) after we integrate with additional > > > >>>AMD gfx > > > >>>drivers. > > > >>> > > > >>>For people who like to review using git, the v2 patch set is located > > > >>>at: > > > >>>http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > > > >>> > > > >>>Written by Oded Gabbayh > > > >> > > > >>So quick comments before i finish going over all patches. There is many > > > >>things that need more documentation espacialy as of right now there is > > > >>no userspace i can go look at. > > > >So quick comments on some of your questions but first of all, thanks for > > > >the time you dedicated to review the code. > > > >> > > > >>There few show stopper, biggest one is gpu memory pinning this is a big > > > >>no, that would need serious arguments for any hope of convincing me on > > > >>that side. > > > >We only do gpu memory pinning for kernel objects. There are no userspace > > > >objects that are pinned on the gpu memory in our driver. If that is the > > > >case, is it still a show stopper ? > > > > > > > >The kernel objects are: > > > >- pipelines (4 per device) > > > >- mqd per hiq (only 1 per device) > > > >- mqd per userspace queue. On KV, we support up to 1K queues per process, > > > >for a total of 512K queues. Each mqd is 151 bytes, but the allocation is > > > >done in 256 alignment. So total *possible* memory is 128MB > > > >- kernel queue (only 1 per device) > > > >- fence address for kernel queue > > > >- runlists for the CP (1 or 2 per device) > > > > > > The main questions here are if it's avoid able to pin down the memory and > > > if > > > the memory is pinned down at driver load, by request from userspace or by > > > anything else. > > > > > > As far as I can see only the "mqd per userspace queue" might be a bit > > > questionable, everything else sounds reasonable. > > > > Aside, i915 perspective again (i.e. how we solved this): When scheduling > > away from contexts we unpin them and put them into the lru. And in the > > shrinker we have a last-ditch callback to switch to a default context > > (since you can't ever have no context once you've started) which means we > > can evict any context object if it's getting in the way. > > So Intel hardware report through some interrupt or some channel when it is > not using a context ? ie kernel side get notification when some user context > is done executing ? Yes, as long as we do the scheduling with the cpu we get interrupts for context switches. The mechanic is already published in the execlist patches currently floating around. We get a special context switch interrupt. But we have this unpin logic already on the current code where we switch contexts through in-line cs commands from the kernel. There we obviously use the normal batch completion events. > The issue with radeon hardware AFAICT is that the hardware do not report any > thing about the userspace context running ie you do not get notification when > a context is not use. Well AFAICT. Maybe hardware do provide that. I'm not sure whether we can do the same trick with the hw scheduler. But then unpinning hw contexts will drain the pipeline anyway, so I guess we can just
[PATCH v2 00/25] AMDKFD kernel driver
On Tue, Jul 22, 2014 at 12:56:13AM +0300, Oded Gabbay wrote: > On 21/07/14 22:28, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 10:23:43PM +0300, Oded Gabbay wrote: > >> On 21/07/14 21:59, Jerome Glisse wrote: > >>> On Mon, Jul 21, 2014 at 09:36:44PM +0300, Oded Gabbay wrote: > On 21/07/14 21:14, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: > >> On 21/07/14 18:54, Jerome Glisse wrote: > >>> On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: > On 21/07/14 16:39, Christian K?nig wrote: > > Am 21.07.2014 14:36, schrieb Oded Gabbay: > >> On 20/07/14 20:46, Jerome Glisse wrote: > >>> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > Forgot to cc mailing list on cover letter. Sorry. > > As a continuation to the existing discussion, here is a v2 patch > series > restructured with a cleaner history and no > totally-different-early-versions > of the code. > > Instead of 83 patches, there are now a total of 25 patches, > where 5 of them > are modifications to radeon driver and 18 of them include only > amdkfd code. > There is no code going away or even modified between patches, > only added. > > The driver was renamed from radeon_kfd to amdkfd and moved to > reside under > drm/radeon/amdkfd. This move was done to emphasize the fact that > this driver > is an AMD-only driver at this point. Having said that, we do > foresee a > generic hsa framework being implemented in the future and in > that case, we > will adjust amdkfd to work within that framework. > > As the amdkfd driver should support multiple AMD gfx drivers, we > want to > keep it as a seperate driver from radeon. Therefore, the amdkfd > code is > contained in its own folder. The amdkfd folder was put under the > radeon > folder because the only AMD gfx driver in the Linux kernel at > this point > is the radeon driver. Having said that, we will probably need to > move it > (maybe to be directly under drm) after we integrate with > additional AMD gfx > drivers. > > For people who like to review using git, the v2 patch set is > located at: > http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > > Written by Oded Gabbayh > >>> > >>> So quick comments before i finish going over all patches. There > >>> is many > >>> things that need more documentation espacialy as of right now > >>> there is > >>> no userspace i can go look at. > >> So quick comments on some of your questions but first of all, > >> thanks for the > >> time you dedicated to review the code. > >>> > >>> There few show stopper, biggest one is gpu memory pinning this is > >>> a big > >>> no, that would need serious arguments for any hope of convincing > >>> me on > >>> that side. > >> We only do gpu memory pinning for kernel objects. There are no > >> userspace > >> objects that are pinned on the gpu memory in our driver. If that > >> is the case, > >> is it still a show stopper ? > >> > >> The kernel objects are: > >> - pipelines (4 per device) > >> - mqd per hiq (only 1 per device) > >> - mqd per userspace queue. On KV, we support up to 1K queues per > >> process, for > >> a total of 512K queues. Each mqd is 151 bytes, but the allocation > >> is done in > >> 256 alignment. So total *possible* memory is 128MB > >> - kernel queue (only 1 per device) > >> - fence address for kernel queue > >> - runlists for the CP (1 or 2 per device) > > > > The main questions here are if it's avoid able to pin down the > > memory and if the > > memory is pinned down at driver load, by request from userspace or > > by anything > > else. > > > > As far as I can see only the "mqd per userspace queue" might be a > > bit > > questionable, everything else sounds reasonable. > > > > Christian. > > Most of the pin downs are done on device initialization. > The "mqd per userspace" is done per userspace queue creation. > However, as I > said, it has an upper limit of 128MB on KV, and considering the 2G > l
[Bug 71812] VDPAU: MPEG-4 ASP Garbling/Corruption
https://bugs.freedesktop.org/show_bug.cgi?id=71812
--- Comment #13 from Fabrice Bellet ---
About this specific bug this time :)
I see a deterministic corruption in your sample too, that seems related to the
presence of sprite frames in the stream, and the result looks like these S
frames are just ignored by the hardware decoder (they are correctly passed to
the mesa level).
For example, an effect of the corruption is the "C" of Captain, that is warped
into something that looks like the euro symbol (?), around 22.5s. Is it similar
to what you observe ? I can provide screenshot if needed.
I tried with VDPAU on radeonsi/uvd, with nouveau, and also with the proprietary
nvidia VDPAU implementation. _All_ implementations have this problem. The
radeon and the nvidia driver just silently drop S frames, and the following
non-S frames are corrupted. The nouveau driver replaces S frame by something
that looks like the rendering of an unintialized vmem buffer.
Also, I can reproduce the _same_ corruption with the ffmpeg software mpeg4
decoder, just by disabling the S-frames handling with this patch:
diff -uNrp ffmpeg-2.1.5.orig/libavcodec/mpeg4videodec.c
ffmpeg-2.1.5/libavcodec/mpeg4videodec.c
--- ffmpeg-2.1.5.orig/libavcodec/mpeg4videodec.c2014-07-21
20:43:58.550548835 +0200
+++ ffmpeg-2.1.5/libavcodec/mpeg4videodec.c2014-07-21 20:43:11.632103072
+0200
@@ -2105,6 +2105,8 @@ static int decode_vop_header(MpegEncCont
}
if(s->pict_type == AV_PICTURE_TYPE_S &&
(s->vol_sprite_usage==STATIC_SPRITE || s->vol_sprite_usage==GMC_SPRITE)){
+ printf("S frame\n");
+ return AVERROR_INVALIDDATA;
if (mpeg4_decode_sprite_trajectory(s, gb) < 0)
return AVERROR_INVALIDDATA;
if(s->sprite_brightness_change) av_log(s->avctx, AV_LOG_ERROR,
"sprite_brightness_change not supported\n");
Are sprite frames supposed to be decoded by the hardware ? The radeon driver
has a sprite-related section in its ruvd_mpeg4 struct definition in
mesa/src/gallium/drivers/radeon/radeon_uvd.h.
--
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/2bb36b23/attachment.html>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 03:43:13PM +0200, Boris BREZILLON wrote: > How about using a list instead of an array ? > This way we can add elements to this list when parsing the EDID. > > Or we can just define a maximum size for the bus_formats array when > retrieving this info from EDID. If I'm correct we have at most 18 bus > formats: > - 3 color formats: >* RGB 4:4:4 >* YCbCr 4:4:4 >* YCbCr 4:4:2 > - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color This starts to worry me. What are we trying to do here - are we trying to encode the connection between the CRTC and the encoder, the encoder and the connector, or the connector and the device? The encoder to connector and connector to device is mostly a function of the interface spec itself (for example, many HDMI encoders take either a RGB or YUV input and can convert it to the HDMI specified colourspaces for transmission over the connector.) If you want to do encoder to connector, what about VGA or some other analogue signalling such as TV composite? It's easy to take this too far... Surely the only one which matters is the CRTC to the encoder - that's certainly true of all the setups I've come across so far. As for that interface, CRTCs I've seen can produce a /wide/ range of different representations. Some CRTCs (eg, AMBA CLCD) produce R, G, B signals scrunched down on to the LSB bits of a LCD data bus (so RGB888 uses 24 bits, RGB444 would use the LSB 12 bits of those 24 - rather than outputting the R4 bits on a subset of the R8 bits.) What about RGB565 - where you have differing number of bits for the green channel from red/blue? Then you have red/blue colour swapping at the CRTC (and similar for YUV) such as on Dove / Armada. Then there are some encoders have the ability to almost arbitarily map their input pins according to whatever you choose (eg, TDA998x). This problem isn't as quite as simple as "this is what EDID gives us" and "these are the number of bits representing a colour". -- FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up according to speedtest.net.
[PATCH V5 05/12] Documentation: Add DT bindings for panel-lvds driver
Hi Thierry, On Mon, Jul 21, 2014 at 1:22 PM, Thierry Reding wrote: > On Fri, Jul 18, 2014 at 12:18:05PM +0530, Ajay kumar wrote: >> Hi Thierry, >> >> Thanks for your comments. >> >> On Fri, Jul 18, 2014 at 4:18 AM, Thierry Reding >> wrote: >> > On Fri, Jul 18, 2014 at 02:20:39AM +0530, Ajay kumar wrote: >> >> +devicetree at vger.kernel.org >> >> >> >> On Fri, Jul 18, 2014 at 2:13 AM, Ajay Kumar >> >> wrote: >> >> > Add DT binding documentation for panel-lvds driver. >> >> > >> >> > Signed-off-by: Ajay Kumar >> >> > --- >> >> > .../devicetree/bindings/panel/panel-lvds.txt | 50 >> >> > >> >> > 1 file changed, 50 insertions(+) >> >> > create mode 100644 >> >> > Documentation/devicetree/bindings/panel/panel-lvds.txt >> >> > >> >> > diff --git a/Documentation/devicetree/bindings/panel/panel-lvds.txt >> >> > b/Documentation/devicetree/bindings/panel/panel-lvds.txt >> >> > new file mode 100644 >> >> > index 000..fdf91da2 >> >> > --- /dev/null >> >> > +++ b/Documentation/devicetree/bindings/panel/panel-lvds.txt >> >> > @@ -0,0 +1,50 @@ >> >> > +panel interface for eDP/lvds panels >> >> > + >> >> > +Required properties: >> >> > + - compatible: "panel-lvds" >> > >> > I think I've said this before. I oppose the addition of this binding. We >> > need to list a device-specific compatible value here, wildcards like >> > this aren't a good choice. And then if we have that compatible value we >> > can move most of the optional properties below into a driver. >> Yes, "panel-lvds" is a wildcard for all lvds panels. >> And since lvds panels from different vendors have different values >> for power_up_delay, delay from video_to_backlight etc, I thought its >> better to pick them up from device tree. > > What I'm saying is that we shouldn't be adding this type of wildcard. > Instead the compatible property needs to list the specific type of panel > and since the driver will match on that specific compatible, the values > for the delays etc. are all implicit and don't have to be listed in > device tree. > >> >> > +Optional properties: >> > >> > If all these properties are optional, then what happens if a device tree >> > node doesn't contain any of them? Doesn't that turn the driver into one >> > big no-op? >> No! We need to provide lcd-supply and backlight-supply. > > What are those? I don't see them described anywhere. > >> >> > + -lcd-enable-gpios: >> >> > + panel LCD poweron GPIO. >> >> > + Indicates which GPIO needs to be powered up as >> >> > output >> >> > + to powerup/enable the switch to the LCD panel. >> >> > + -backlight-enable-gpios: >> >> > + panel LED enable GPIO. >> >> > + Indicates which GPIO needs to be powered up as >> >> > output >> >> > + to enable the backlight. >> > >> > I've also said before that this really belongs in a backlight driver. >> > Chances are that you'll want to have a way to set the brightness of the >> > backlight as well, so simply an enable GPIO won't be good enough. >> Ok. I can handle this in backlight driver itself (with some minor glitches). > > You can make it work without glitches as well and we've discussed this > before. > >> But, how do I map bridge functions to panel functions now? >> The bridge supports (pre_enable, enable, disable and post_disable) which I >> map >> with (prepare, enable, disable and unprepare) of the panel, using a sample >> layer >> called bridge to panel_binder. >> Moving out the backlight control from panel means I really don't need >> those extra >> panel calls(prepare and unprepare)! >> Then how to distribute 2 panel calls(enable and disable) across 4 bridge >> calls? > > The .prepare() and .unprepare() functions are useful irrespective of > backlight control. What you want to separate is powering up the panel > from enabling the backlight. So .prepare() could be what's doing the > power up of the panel (and possibly other initialization required to > make it work) and .enable() could be as simple as turning on the > backlight. > > What I'm saying is rather than use a backlight-enable-gpios property in > the panel, that property should be part of the backlight device and the > GPIO can then be toggled by the backlight driver when the backlight is > enabled. > >> >> > + -panel-prepare-delay: >> >> > + delay value in ms required for panel_prepare process >> >> > + Delay in ms needed for the panel LCD unit to >> >> > + powerup completely. >> >> > + ex: delay needed till eDP panel throws HPD. >> >> > + delay needed so that we cans tart reading >> >> > edid. >> > >> > If the panel signals HPD then we don't need this delay at all and we >> > should just wait for HPD instead. >> Not always for HPD, we need to wait for EDID module as well. > > I always thought that HPD signal
[RESEND PATCH V5 03/12] drm/exynos: dp: modify driver to support drm_panel
Hi Thierry,
On Mon, Jul 21, 2014 at 1:44 PM, Thierry Reding
wrote:
> On Fri, Jul 18, 2014 at 02:13:49AM +0530, Ajay Kumar wrote:
>> Add drm_panel controls to support powerup/down of the
>> eDP panel, if one is present at the sink side.
>>
>> Signed-off-by: Ajay Kumar
>> ---
>> .../devicetree/bindings/video/exynos_dp.txt|2 +
>> drivers/gpu/drm/exynos/Kconfig |1 +
>> drivers/gpu/drm/exynos/exynos_dp_core.c| 45
>>
>> drivers/gpu/drm/exynos/exynos_dp_core.h|2 +
>> 4 files changed, 41 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt
>> b/Documentation/devicetree/bindings/video/exynos_dp.txt
>> index 53dbccf..c029a09 100644
>> --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
>> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
>> @@ -51,6 +51,8 @@ Required properties for dp-controller:
>> LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
>> - display-timings: timings for the connected panel as described by
>> Documentation/devicetree/bindings/video/display-timing.txt
>> + -edp-panel:
>> + phandle for the edp/lvds drm_panel node.
>
> There's really no need for the edp- prefix here. Also please don't use
> drm_panel in the binding description since it makes no sense outside of
> DRM (and bindings need to be OS agnostic).
>
> Also: "eDP" and "LVDS"
Ok. Will fix this.
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> index a94b114..b3d0d9b 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> @@ -28,6 +28,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #include "exynos_drm_drv.h"
>> @@ -1029,6 +1030,9 @@ static int exynos_dp_create_connector(struct
>> exynos_drm_display *display,
>> drm_connector_register(connector);
>> drm_mode_connector_attach_encoder(connector, encoder);
>>
>> + if (dp->edp_panel)
>> + drm_panel_attach(dp->edp_panel, &dp->connector);
>
> This function can fail, so you really need to do some error-checking
> here.
Ok.
>> @@ -1063,11 +1067,13 @@ static void exynos_dp_poweron(struct
>> exynos_dp_device *dp)
>> if (dp->dpms_mode == DRM_MODE_DPMS_ON)
>> return;
>>
>> + drm_panel_prepare(dp->edp_panel);
>> clk_prepare_enable(dp->clock);
>> exynos_dp_phy_init(dp);
>> exynos_dp_init_dp(dp);
>> enable_irq(dp->irq);
>> exynos_dp_setup(dp);
>> + drm_panel_enable(dp->edp_panel);
>
> These two as well. clk_prepare_enable() can also fail by the way.
>
> exynos_dp_init_dp() can also fail theoretically (it returns int) but
> never returns anything other than 0, so it could just as well return
> void.
Ok. Will fix this.
>> @@ -1075,10 +1081,12 @@ static void exynos_dp_poweroff(struct
>> exynos_dp_device *dp)
>> if (dp->dpms_mode != DRM_MODE_DPMS_ON)
>> return;
>>
>> + drm_panel_disable(dp->edp_panel);
>> disable_irq(dp->irq);
>> flush_work(&dp->hotplug_work);
>> exynos_dp_phy_exit(dp);
>> clk_disable_unprepare(dp->clock);
>> + drm_panel_unprepare(dp->edp_panel);
>> }
>
> And these two also.
Ok.
>> static void exynos_dp_dpms(struct exynos_drm_display *display, int mode)
>> @@ -1209,8 +1217,17 @@ err:
>> static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp)
>> {
>> int ret;
>> + struct device_node *videomode_parent;
>> +
>> + /* Receive video timing info from panel node
>> + * if there is a panel node
>> + */
>> + if (dp->panel_node)
>> + videomode_parent = dp->panel_node;
>> + else
>> + videomode_parent = dp->dev->of_node;
>>
>> - ret = of_get_videomode(dp->dev->of_node, &dp->panel.vm,
>> + ret = of_get_videomode(videomode_parent, &dp->panel.vm,
>
> You shouldn't be grabbing the video timings from some random node like
> this. Instead you should use the DRM panel's .get_modes() function to
> query the supported modes. The panel may not (in fact, should not, at
> least under most circumstances) define video timings in the device tree.
Right, I am planning to use panel-simple driver by adding drm_display_mode
structure for the lvds panels. So, I would not need this change.
>> @@ -1341,6 +1353,21 @@ static int exynos_dp_probe(struct platform_device
>> *pdev)
>> if (ret)
>> return ret;
>>
>> + dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
>> + GFP_KERNEL);
>> + if (!dp)
>> + return -ENOMEM;
>> +
>> + dp->panel_node = of_parse_phandle(dev->of_node, "edp-panel", 0);
>
> There should be no need to store the struct device_node * obtained here
> in the context like this.
>
>> + if (dp->panel_node) {
>> + dp->edp_panel = of_drm_find_
[pull] radeon drm-fixes-3.16
Hi Dave, Another fixes pull for radeon. This one is a little bigger than I'd like, but fixes a stability issue with GPUVM that led to GPU hangs that was introduced in 3.15. Also included is a system hang/reboot regression fix for dpm on TN/RL hardware. The following changes since commit e898c791e1a4c27fa1f221058b29b0ad06ddf8b0: Merge tag 'drm-intel-fixes-2014-07-18' of git://anongit.freedesktop.org/drm-intel (2014-07-19 16:48:38 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-fixes-3.16 for you to fetch changes up to 730a336c33a3398d65896e8ee3ef9f5679fe30a9: drm/radeon/TN: only enable bapm on MSI systems (2014-07-21 13:17:39 -0400) Alex Deucher (1): drm/radeon/TN: only enable bapm on MSI systems Christian K?nig (3): drm/radeon: let's use GB for vm_size (v2) drm/radeon: fix handling of radeon_vm_bo_rmv v3 drm/radeon: fix VM IB handling drivers/gpu/drm/radeon/radeon.h| 15 -- drivers/gpu/drm/radeon/radeon_cs.c | 20 +++- drivers/gpu/drm/radeon/radeon_device.c | 22 - drivers/gpu/drm/radeon/radeon_drv.c| 4 +- drivers/gpu/drm/radeon/radeon_kms.c| 26 +-- drivers/gpu/drm/radeon/radeon_vm.c | 83 -- drivers/gpu/drm/radeon/trinity_dpm.c | 15 +++--- 7 files changed, 122 insertions(+), 63 deletions(-)
[RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining
Hi Thierry, On Mon, Jul 21, 2014 at 1:52 PM, Thierry Reding wrote: > On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote: > [...] >> Also, remove the drm_connector implementation from ptn3460, >> since the same is implemented using panel_binder. > > I think that's a step backwards. In fact I think the panel-bridge binder > driver shouldn't be needed at all. At least not for now. We have a very > limited number of bridge drivers, so it shouldn't hurt at this stage to > implement the connector instantiation within each driver. Once we have > more bridge drivers, and therefore a better understanding of what they > need, we can always add something like the panel-binder (though I think > it should be library code, similar to the drm_encoder_helper_add() API, > rather than this kind of self-contained object). panel_binder was needed to wrap around panel as a bridge, and this was in turn needed because people wanted to represent a bridge+panel combo as a chain of bridges. So, if we choose to drop panel_binder, we choose to drop the idea of chaining the bridges, and end up calling drm_panel functions directly from individual bridges. And, the bridge will implement the connector functions as you suggested. I am okay with this, if Daniel/Rob don't have an issue with this. Ajay
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 03:39:09PM +0200, Christian K?nig wrote: > Am 21.07.2014 14:36, schrieb Oded Gabbay: > >On 20/07/14 20:46, Jerome Glisse wrote: > >>On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > >>>Forgot to cc mailing list on cover letter. Sorry. > >>> > >>>As a continuation to the existing discussion, here is a v2 patch series > >>>restructured with a cleaner history and no > >>>totally-different-early-versions > >>>of the code. > >>> > >>>Instead of 83 patches, there are now a total of 25 patches, where 5 of > >>>them > >>>are modifications to radeon driver and 18 of them include only amdkfd > >>>code. > >>>There is no code going away or even modified between patches, only > >>>added. > >>> > >>>The driver was renamed from radeon_kfd to amdkfd and moved to reside > >>>under > >>>drm/radeon/amdkfd. This move was done to emphasize the fact that this > >>>driver > >>>is an AMD-only driver at this point. Having said that, we do foresee a > >>>generic hsa framework being implemented in the future and in that > >>>case, we > >>>will adjust amdkfd to work within that framework. > >>> > >>>As the amdkfd driver should support multiple AMD gfx drivers, we want > >>>to > >>>keep it as a seperate driver from radeon. Therefore, the amdkfd code is > >>>contained in its own folder. The amdkfd folder was put under the radeon > >>>folder because the only AMD gfx driver in the Linux kernel at this > >>>point > >>>is the radeon driver. Having said that, we will probably need to move > >>>it > >>>(maybe to be directly under drm) after we integrate with additional > >>>AMD gfx > >>>drivers. > >>> > >>>For people who like to review using git, the v2 patch set is located > >>>at: > >>>http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > >>> > >>>Written by Oded Gabbayh > >> > >>So quick comments before i finish going over all patches. There is many > >>things that need more documentation espacialy as of right now there is > >>no userspace i can go look at. > >So quick comments on some of your questions but first of all, thanks for > >the time you dedicated to review the code. > >> > >>There few show stopper, biggest one is gpu memory pinning this is a big > >>no, that would need serious arguments for any hope of convincing me on > >>that side. > >We only do gpu memory pinning for kernel objects. There are no userspace > >objects that are pinned on the gpu memory in our driver. If that is the > >case, is it still a show stopper ? > > > >The kernel objects are: > >- pipelines (4 per device) > >- mqd per hiq (only 1 per device) > >- mqd per userspace queue. On KV, we support up to 1K queues per process, > >for a total of 512K queues. Each mqd is 151 bytes, but the allocation is > >done in 256 alignment. So total *possible* memory is 128MB > >- kernel queue (only 1 per device) > >- fence address for kernel queue > >- runlists for the CP (1 or 2 per device) > > The main questions here are if it's avoid able to pin down the memory and if > the memory is pinned down at driver load, by request from userspace or by > anything else. > > As far as I can see only the "mqd per userspace queue" might be a bit > questionable, everything else sounds reasonable. Aside, i915 perspective again (i.e. how we solved this): When scheduling away from contexts we unpin them and put them into the lru. And in the shrinker we have a last-ditch callback to switch to a default context (since you can't ever have no context once you've started) which means we can evict any context object if it's getting in the way. We must do that since the contexts have to be in global gtt, which is shared for scanouts. So fragmenting that badly with lots of context objects and other stuff is a no-go, since that means we'll start to fail pageflips. I don't know whether ttm has a ready-made concept for such opportunistically pinned stuff. I guess you could wire up the "switch to dflt context" action to the evict/move function if ttm wants to get rid of the currently used hw context. Oh and: This is another reason for letting the kernel schedule contexts, since you can't do this defrag trick if the gpu does all the scheduling itself. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 0/25] Replace DEFINE_PCI_DEVICE_TABLE macro use
[+cc Jingoo] On Fri, Jul 18, 2014 at 12:50 PM, James Bottomley wrote: > On Fri, 2014-07-18 at 11:17 -0700, Greg KH wrote: >> On Fri, Jul 18, 2014 at 09:54:32AM -0700, James Bottomley wrote: >> > On Fri, 2014-07-18 at 09:43 -0700, Greg KH wrote: >> > > On Fri, Jul 18, 2014 at 12:22:13PM -0400, John W. Linville wrote: >> > > > On Fri, Jul 18, 2014 at 05:26:47PM +0200, Benoit Taine wrote: >> > > > > We should prefer `const struct pci_device_id` over >> > > > > `DEFINE_PCI_DEVICE_TABLE` to meet kernel coding style guidelines. >> > > > > This issue was reported by checkpatch. >> > > > >> > > > Honestly, I prefer the macro -- it stands-out more. Maybe the style >> > > > guidelines and/or checkpatch should change instead? >> > > >> > > The macro is horrid, no other bus has this type of thing just to save a >> > > few characters in typing >> > >> > OK, so this is the macro: >> > >> > #define DEFINE_PCI_DEVICE_TABLE(_table) \ >> > const struct pci_device_id _table[] >> > >> > Could you explain what's so horrible? >> > >> > The reason it's useful today is that people forget the const (and >> > sometimes the [] making it a true table instead of a pointer). If you >> > use the DEFINE_PCI_DEVICE_TABLE macro, the compile breaks if you use it >> > wrongly (good) and you automatically get the correct annotations. >> >> We have almost 1000 more uses of the non-macro version than the "macro" >> version in the kernel today: >> $ git grep -w DEFINE_PCI_DEVICE_TABLE | wc -l >> 262 >> $ git grep "const struct pci_device_id" | wc -l >> 1254 >> >> My big complaint is that we need to be consistant, either pick one or >> the other and stick to it. As the macro is the least used, it's easiest >> to fix up, and it also is more consistant with all other kernel >> subsystems which do not have such a macro. > > I've a weak preference for consistency, but not at the expense of > hundreds of patches churning the kernel to remove an innocuous macro. > Churn costs time and effort. > >> As there is no need for the __init macro mess anymore, there's no real >> need for the DEFINE_PCI_DEVICE_TABLE macro either. I think checkpatch >> will catch the use of non-const users for the id table already today, it >> catches lots of other uses like this already. >> >> > > , so why should PCI be "special" in this regard >> > > anymore? >> > >> > I think the PCI usage dwarfs most other bus types now, so you could turn >> > the question around. However, I don't think majority voting is a good >> > guide to best practise; lets debate the merits for their own sake. >> >> Not really "dwarf", USB is close with over 700 such structures: >> $ git grep "const struct usb_device_id" | wc -l >> 725 >> >> And i2c is almost just as big as PCI: >> $ git grep "const struct i2c_device_id" | wc -l >> 1223 >> >> So again, this macro is not consistent with the majority of PCI drivers, >> nor with any other type of "device id" declaration in the kernel, which >> is why I feel it should be removed. >> >> And finally, the PCI documentation itself says to not use this macro, so >> this isn't a "new" thing. From Documentation/PCI/pci.txt: >> >> The ID table is an array of struct pci_device_id entries ending with an >> all-zero entry. Definitions with static const are generally preferred. >> Use of the deprecated macro DEFINE_PCI_DEVICE_TABLE should be avoided. >> >> That wording went into the file last December, when we last talked about >> this and everyone in that discussion agreed to remove the macro for the >> above reasons. >> >> Consistency matters. > > In this case, I don't think it does that much ... a cut and paste either > way (from a macro or non-macro based driver) yields correct code. Since > there's no bug here and no apparent way to misuse the macro, why bother? > > Anyway, it's PCI code ... let the PCI maintainer decide. However, if he > does want this do it as one big bang patch via either the PCI or Trivial > tree (latter because Ji?? has experience doing this, but the former > might be useful so the decider feels the pain ...) I don't feel strongly either way, so I guess I'm OK with this, and in the spirit of feeling the pain, I'm willing to handle it. Jingoo proposed similar patches, so it might be nice to give him some credit. Benoit, how about if you wait until about half-way through the merge window after v3.16 releases, generate an up-to-date single patch, and post that? Then we can try to get it in before v3.17-rc1 to minimize merge hassles. Bjorn
[PATCH v2 00/25] AMDKFD kernel driver
On 21/07/14 16:39, Christian K?nig wrote: > Am 21.07.2014 14:36, schrieb Oded Gabbay: >> On 20/07/14 20:46, Jerome Glisse wrote: >>> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: Forgot to cc mailing list on cover letter. Sorry. As a continuation to the existing discussion, here is a v2 patch series restructured with a cleaner history and no totally-different-early-versions of the code. Instead of 83 patches, there are now a total of 25 patches, where 5 of them are modifications to radeon driver and 18 of them include only amdkfd code. There is no code going away or even modified between patches, only added. The driver was renamed from radeon_kfd to amdkfd and moved to reside under drm/radeon/amdkfd. This move was done to emphasize the fact that this driver is an AMD-only driver at this point. Having said that, we do foresee a generic hsa framework being implemented in the future and in that case, we will adjust amdkfd to work within that framework. As the amdkfd driver should support multiple AMD gfx drivers, we want to keep it as a seperate driver from radeon. Therefore, the amdkfd code is contained in its own folder. The amdkfd folder was put under the radeon folder because the only AMD gfx driver in the Linux kernel at this point is the radeon driver. Having said that, we will probably need to move it (maybe to be directly under drm) after we integrate with additional AMD gfx drivers. For people who like to review using git, the v2 patch set is located at: http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 Written by Oded Gabbayh >>> >>> So quick comments before i finish going over all patches. There is many >>> things that need more documentation espacialy as of right now there is >>> no userspace i can go look at. >> So quick comments on some of your questions but first of all, thanks for the >> time you dedicated to review the code. >>> >>> There few show stopper, biggest one is gpu memory pinning this is a big >>> no, that would need serious arguments for any hope of convincing me on >>> that side. >> We only do gpu memory pinning for kernel objects. There are no userspace >> objects that are pinned on the gpu memory in our driver. If that is the case, >> is it still a show stopper ? >> >> The kernel objects are: >> - pipelines (4 per device) >> - mqd per hiq (only 1 per device) >> - mqd per userspace queue. On KV, we support up to 1K queues per process, for >> a total of 512K queues. Each mqd is 151 bytes, but the allocation is done in >> 256 alignment. So total *possible* memory is 128MB >> - kernel queue (only 1 per device) >> - fence address for kernel queue >> - runlists for the CP (1 or 2 per device) > > The main questions here are if it's avoid able to pin down the memory and if > the > memory is pinned down at driver load, by request from userspace or by anything > else. > > As far as I can see only the "mqd per userspace queue" might be a bit > questionable, everything else sounds reasonable. > > Christian. Most of the pin downs are done on device initialization. The "mqd per userspace" is done per userspace queue creation. However, as I said, it has an upper limit of 128MB on KV, and considering the 2G local memory, I think it is OK. The runlists are also done on userspace queue creation/deletion, but we only have 1 or 2 runlists per device, so it is not that bad. Oded > >>> >>> It might be better to add a drivers/gpu/drm/amd directory and add common >>> stuff there. >>> >>> Given that this is not intended to be final HSA api AFAICT then i would >>> say this far better to avoid the whole kfd module and add ioctl to radeon. >>> This would avoid crazy communication btw radeon and kfd. >>> >>> The whole aperture business needs some serious explanation. Especialy as >>> you want to use userspace address there is nothing to prevent userspace >>> program from allocating things at address you reserve for lds, scratch, >>> ... only sane way would be to move those lds, scratch inside the virtual >>> address reserved for kernel (see kernel memory map). >>> >>> The whole business of locking performance counter for exclusive per process >>> access is a big NO. Which leads me to the questionable usefullness of user >>> space command ring. >> That's like saying: "Which leads me to the questionable usefulness of HSA". I >> find it analogous to a situation where a network maintainer nacking a driver >> for a network card, which is slower than a different network card. Doesn't >> seem reasonable this situation is would happen. He would still put both the >> drivers in the kernel because people want to use the H/W and its features. >> So, >> I don't think this is a valid reason to NACK the driver. >> >>> I only see issues with that. First and foremost i would >>> need to see solid figures that kernel ioctl or syscal
[PATCH V5 00/12] drm/exynos: few patches to enhance bridge chip support
Hi Inki, On Mon, Jul 21, 2014 at 1:21 PM, Inki Dae wrote: > On 2014? 07? 18? 05:43, Ajay Kumar wrote: >> This series is based on exynos-drm-next branch of Inki Dae's tree at: >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> This patchset also consolidates various inputs from the drm community >> regarding the bridge chaining concept: >> (1) [RFC V2 0/3] drm/bridge: panel and chaining >> http://www.spinics.net/lists/linux-samsung-soc/msg30160.html >> (2) [RFC V3 0/3] drm/bridge: panel and chaining >> http://www.spinics.net/lists/linux-samsung-soc/msg30507.html >> >> I have tested this after adding few DT changes for exynos5250-snow, >> exynos5420-peach-pit and exynos5800-peach-pi boards. >> >> The V4 series of this particular patchset was also tested by: >> Rahul Sharma >> Javier Martinez Canillas >> >> Changes since V2: >> -- Address comments from Jingoo Han for ps8622 driver >> -- Address comments from Daniel, Rob and Thierry regarding >> bridge chaining >> -- Address comments from Thierry regarding the names for >> new drm_panel functions >> >> Changes since V3: >> -- Remove hotplug based initialization of exynos_dp >> -- Make exynos_dp work directly with drm_panel, remove >> dependency on panel_binder >> -- Minor cleanups in panel_binder and panel_lvds driver >> >> Changes since V4: >> -- Use gpiod interface for panel-lvds and ps8622 drivers. >> -- Address comments from Javier. >> -- Fix compilation issues when PANEL_BINDER is selected as module. >> -- Split Documentation patches from driver patches. >> -- Rebase on top of the tree. > > Hi Ajay, > > Thanks for your contribution. > > I am reviewing your patch series. Sorry for late. Below is my comment. > > How about using graph concept to bind eDP, LVDS bridge, and Panel? Your > patch tries to bind bridge driver using find_bridge function, and this > function tries to find bridge device node directly using > of_find_compatible_node() again, which in turn make eDP driver to add > all codes related to all bridge devices including all relevant bridge > headers: i.e., if there are five bridge devices then eDP driver should > try to find all of them. That is really not good. I agree. > So my opinion is to define the relationship between eDP, LVDS, and Panel > using graph concept: eDP context would have panel and bridge object > according to graph definition of device tree. > > As a result, eDP context has already bridge_chain object for lvds bridge > device and panel_binder->bridge object in exynos_drm_attach_lcd_bridge > function context so it can add bridge_chain object to > panel_binder->bridge as is there. I think lvds bridge device drivers > could follow Linux driver model with this approach. Sorry, I didn't quite understand the graph concept. Already Thierry suggested of having a common repository for list of supported bridges. That way, we can move out bridge detection stuff from exynos_dp to a common file. Ajay > Rob, it seems to need at least your ACK so that I can merge this patch > series to exynos-drm-next. > > Thanks, > Inki Dae > >> >> Ajay Kumar (9): >> [RESEND PATCH V5 01/12] drm/exynos: Move DP setup out of hotplug workqueue >> [RESEND PATCH V5 02/12] drm/panel: add prepare and unprepare routines >> [RESEND PATCH V5 03/12] drm/exynos: dp: modify driver to support drm_panel >> [PATCH V5 04/12] drm/panel: Add driver for lvds/edp based panels >> [PATCH V5 05/12] Documentation: Add DT bindings for panel-lvds driver >> [RESEND PATCH V5 06/12] drm/bridge: add helper functions to support bridge >> chain >> [PATCH V5 07/12] drm/bridge: Add a driver which binds drm_bridge with >> drm_panel >> [RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining >> [RESEND PATCH V5 09/12] drm/exynos: dp: create bridge chain using ptn3460 >> and panel_binder >> >> Vincent Palatin (2): >> [PATCH V5 10/12] Documentation: Add DT bindings for ps8622/ps8625 bridge >> driver >> [PATCH V5 11/12] drm/bridge: Add ps8622/ps8625 bridge driver >> >> Rahul Sharma (1): >> [RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP >> driver >> >> .../devicetree/bindings/drm/bridge/ps8622.txt | 21 + >> .../devicetree/bindings/panel/panel-lvds.txt | 50 ++ >> .../devicetree/bindings/video/exynos_dp.txt|2 + >> drivers/gpu/drm/bridge/Kconfig | 15 + >> drivers/gpu/drm/bridge/Makefile|2 + >> drivers/gpu/drm/bridge/panel_binder.c | 193 >> drivers/gpu/drm/bridge/ps8622.c| 476 >> >> drivers/gpu/drm/bridge/ptn3460.c | 137 +- >> drivers/gpu/drm/exynos/Kconfig |1 + >> drivers/gpu/drm/exynos/exynos_dp_core.c| 87 +++- >> drivers/gpu/drm/exynos/exynos_dp_core.h|2 + >> drivers/
[RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP driver
On Mon, Jul 21, 2014 at 12:40 PM, Thierry Reding
wrote:
> On Fri, Jul 18, 2014 at 02:13:58AM +0530, Ajay Kumar wrote:
>> From: Rahul Sharma
>>
>> This patch adds ps8622 lvds bridge discovery code to the dp driver.
>>
>> Signed-off-by: Rahul Sharma
>> Signed-off-by: Ajay Kumar
>> ---
>> drivers/gpu/drm/exynos/exynos_dp_core.c |5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> index 0ca6256..82e2942 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> @@ -31,6 +31,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "exynos_drm_drv.h"
>> #include "exynos_dp_core.h"
>> @@ -999,6 +1000,10 @@ static int exynos_drm_attach_lcd_bridge(struct
>> exynos_dp_device *dp,
>> if (find_bridge("nxp,ptn3460", &bridge)) {
>> bridge_chain = ptn3460_init(dp->drm_dev, encoder,
>> bridge.client,
>> bridge.node);
>> + } else if (find_bridge("parade,ps8622", &bridge) ||
>> + find_bridge("parade,ps8625", &bridge)) {
>> + bridge_chain = ps8622_init(dp->drm_dev, encoder, bridge.client,
>> + bridge.node);
>> }
>
> We really ought to be adding some sort of registry at some point.
> Otherwise every driver that wants to use bridges needs to come up with a
> similar set of helpers to instantiate them.
>
> Also you're making this driver depend on (now) two bridges, whereas it
> really shouldn't matter which exact types it supports. Bridges should be
> exposed via a generic interface.
How about moving out the find_bridge() function into a common header file,
and also supporting the list of bridge_init declarations in the same file?
We get bridge chip node from phandle, and then pass the same node
to find_bridge() which searches the list using of_device_is_compatible()
to call the appropriate bridge_init function?
Ajay
[RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining
On 2014? 07? 18? 05:43, Ajay Kumar wrote:
> Modify the driver to invoke callbacks for the next bridge
> in the bridge chain.
> Also, remove the drm_connector implementation from ptn3460,
> since the same is implemented using panel_binder.
>
> Signed-off-by: Ajay Kumar
> ---
> drivers/gpu/drm/bridge/ptn3460.c| 137
> +--
> drivers/gpu/drm/exynos/exynos_dp_core.c | 16 ++--
> include/drm/bridge/ptn3460.h| 15 ++--
> 3 files changed, 39 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ptn3460.c
> b/drivers/gpu/drm/bridge/ptn3460.c
> index d466696..5fe16c6 100644
> --- a/drivers/gpu/drm/bridge/ptn3460.c
> +++ b/drivers/gpu/drm/bridge/ptn3460.c
> @@ -34,37 +34,15 @@
> #define PTN3460_EDID_SRAM_LOAD_ADDR 0x85
>
> struct ptn3460_bridge {
> - struct drm_connector connector;
> struct i2c_client *client;
> struct drm_encoder *encoder;
> struct drm_bridge *bridge;
> - struct edid *edid;
> int gpio_pd_n;
> int gpio_rst_n;
> u32 edid_emulation;
> bool enabled;
> };
>
> -static int ptn3460_read_bytes(struct ptn3460_bridge *ptn_bridge, char addr,
> - u8 *buf, int len)
> -{
> - int ret;
> -
> - ret = i2c_master_send(ptn_bridge->client, &addr, 1);
> - if (ret <= 0) {
> - DRM_ERROR("Failed to send i2c command, ret=%d\n", ret);
> - return ret;
> - }
> -
> - ret = i2c_master_recv(ptn_bridge->client, buf, len);
> - if (ret <= 0) {
> - DRM_ERROR("Failed to recv i2c data, ret=%d\n", ret);
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> static int ptn3460_write_byte(struct ptn3460_bridge *ptn_bridge, char addr,
> char val)
> {
> @@ -126,6 +104,8 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
> gpio_set_value(ptn_bridge->gpio_rst_n, 1);
> }
>
> + drm_next_bridge_pre_enable(bridge);
> +
> /*
>* There's a bug in the PTN chip where it falsely asserts hotplug before
>* it is fully functional. We're forced to wait for the maximum start up
> @@ -142,6 +122,7 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge)
>
> static void ptn3460_enable(struct drm_bridge *bridge)
> {
> + drm_next_bridge_enable(bridge);
> }
>
> static void ptn3460_disable(struct drm_bridge *bridge)
> @@ -153,6 +134,8 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>
> ptn_bridge->enabled = false;
>
> + drm_next_bridge_disable(bridge);
> +
> if (gpio_is_valid(ptn_bridge->gpio_rst_n))
> gpio_set_value(ptn_bridge->gpio_rst_n, 1);
>
> @@ -162,6 +145,7 @@ static void ptn3460_disable(struct drm_bridge *bridge)
>
> static void ptn3460_post_disable(struct drm_bridge *bridge)
> {
> + drm_next_bridge_post_disable(bridge);
> }
>
> void ptn3460_bridge_destroy(struct drm_bridge *bridge)
> @@ -173,6 +157,9 @@ void ptn3460_bridge_destroy(struct drm_bridge *bridge)
> gpio_free(ptn_bridge->gpio_pd_n);
> if (gpio_is_valid(ptn_bridge->gpio_rst_n))
> gpio_free(ptn_bridge->gpio_rst_n);
> +
> + drm_next_bridge_destroy(bridge);
> +
> /* Nothing else to free, we've got devm allocated memory */
> }
>
> @@ -184,81 +171,10 @@ struct drm_bridge_funcs ptn3460_bridge_funcs = {
> .destroy = ptn3460_bridge_destroy,
> };
>
> -int ptn3460_get_modes(struct drm_connector *connector)
> -{
> - struct ptn3460_bridge *ptn_bridge;
> - u8 *edid;
> - int ret, num_modes;
> - bool power_off;
> -
> - ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
> -
> - if (ptn_bridge->edid)
> - return drm_add_edid_modes(connector, ptn_bridge->edid);
> -
> - power_off = !ptn_bridge->enabled;
> - ptn3460_pre_enable(ptn_bridge->bridge);
> -
> - edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> - if (!edid) {
> - DRM_ERROR("Failed to allocate edid\n");
> - return 0;
> - }
> -
> - ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid,
> - EDID_LENGTH);
> - if (ret) {
> - kfree(edid);
> - num_modes = 0;
> - goto out;
> - }
> -
> - ptn_bridge->edid = (struct edid *)edid;
> - drm_mode_connector_update_edid_property(connector, ptn_bridge->edid);
> -
> - num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
> -
> -out:
> - if (power_off)
> - ptn3460_disable(ptn_bridge->bridge);
> -
> - return num_modes;
> -}
> -
> -struct drm_encoder *ptn3460_best_encoder(struct drm_connector *connector)
> -{
> - struct ptn3460_bridge *ptn_bridge;
> -
> - ptn_bridge = container_of(connector, struct ptn3460_bridge, connector);
> -
> - return ptn_bridge->encoder;
> -}
> -
> -struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = {
> - .get_modes = ptn3460_get_modes,
> -
[PATCH V5 00/12] drm/exynos: few patches to enhance bridge chip support
On 2014? 07? 18? 05:43, Ajay Kumar wrote: > This series is based on exynos-drm-next branch of Inki Dae's tree at: > git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > > This patchset also consolidates various inputs from the drm community > regarding the bridge chaining concept: > (1) [RFC V2 0/3] drm/bridge: panel and chaining > http://www.spinics.net/lists/linux-samsung-soc/msg30160.html > (2) [RFC V3 0/3] drm/bridge: panel and chaining > http://www.spinics.net/lists/linux-samsung-soc/msg30507.html > > I have tested this after adding few DT changes for exynos5250-snow, > exynos5420-peach-pit and exynos5800-peach-pi boards. > > The V4 series of this particular patchset was also tested by: > Rahul Sharma > Javier Martinez Canillas > > Changes since V2: > -- Address comments from Jingoo Han for ps8622 driver > -- Address comments from Daniel, Rob and Thierry regarding > bridge chaining > -- Address comments from Thierry regarding the names for > new drm_panel functions > > Changes since V3: > -- Remove hotplug based initialization of exynos_dp > -- Make exynos_dp work directly with drm_panel, remove > dependency on panel_binder > -- Minor cleanups in panel_binder and panel_lvds driver > > Changes since V4: > -- Use gpiod interface for panel-lvds and ps8622 drivers. > -- Address comments from Javier. > -- Fix compilation issues when PANEL_BINDER is selected as module. > -- Split Documentation patches from driver patches. > -- Rebase on top of the tree. Hi Ajay, Thanks for your contribution. I am reviewing your patch series. Sorry for late. Below is my comment. How about using graph concept to bind eDP, LVDS bridge, and Panel? Your patch tries to bind bridge driver using find_bridge function, and this function tries to find bridge device node directly using of_find_compatible_node() again, which in turn make eDP driver to add all codes related to all bridge devices including all relevant bridge headers: i.e., if there are five bridge devices then eDP driver should try to find all of them. That is really not good. So my opinion is to define the relationship between eDP, LVDS, and Panel using graph concept: eDP context would have panel and bridge object according to graph definition of device tree. As a result, eDP context has already bridge_chain object for lvds bridge device and panel_binder->bridge object in exynos_drm_attach_lcd_bridge function context so it can add bridge_chain object to panel_binder->bridge as is there. I think lvds bridge device drivers could follow Linux driver model with this approach. Rob, it seems to need at least your ACK so that I can merge this patch series to exynos-drm-next. Thanks, Inki Dae > > Ajay Kumar (9): > [RESEND PATCH V5 01/12] drm/exynos: Move DP setup out of hotplug workqueue > [RESEND PATCH V5 02/12] drm/panel: add prepare and unprepare routines > [RESEND PATCH V5 03/12] drm/exynos: dp: modify driver to support drm_panel > [PATCH V5 04/12] drm/panel: Add driver for lvds/edp based panels > [PATCH V5 05/12] Documentation: Add DT bindings for panel-lvds driver > [RESEND PATCH V5 06/12] drm/bridge: add helper functions to support bridge > chain > [PATCH V5 07/12] drm/bridge: Add a driver which binds drm_bridge with > drm_panel > [RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining > [RESEND PATCH V5 09/12] drm/exynos: dp: create bridge chain using ptn3460 > and panel_binder > > Vincent Palatin (2): > [PATCH V5 10/12] Documentation: Add DT bindings for ps8622/ps8625 bridge > driver > [PATCH V5 11/12] drm/bridge: Add ps8622/ps8625 bridge driver > > Rahul Sharma (1): > [RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP > driver > > .../devicetree/bindings/drm/bridge/ps8622.txt | 21 + > .../devicetree/bindings/panel/panel-lvds.txt | 50 ++ > .../devicetree/bindings/video/exynos_dp.txt|2 + > drivers/gpu/drm/bridge/Kconfig | 15 + > drivers/gpu/drm/bridge/Makefile|2 + > drivers/gpu/drm/bridge/panel_binder.c | 193 > drivers/gpu/drm/bridge/ps8622.c| 476 > > drivers/gpu/drm/bridge/ptn3460.c | 137 +- > drivers/gpu/drm/exynos/Kconfig |1 + > drivers/gpu/drm/exynos/exynos_dp_core.c| 87 +++- > drivers/gpu/drm/exynos/exynos_dp_core.h|2 + > drivers/gpu/drm/panel/Kconfig | 10 + > drivers/gpu/drm/panel/Makefile |1 + > drivers/gpu/drm/panel/panel-lvds.c | 268 +++ > include/drm/bridge/panel_binder.h | 44 ++ > include/drm/bridge/ps8622.h| 41 ++ > include/drm/bridge/ptn3460.h | 15 +- > include/drm/drm_crtc.h
[RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP driver
t; > static int foo_probe(...)
> > {
> > struct foo_bridge *bridge;
> > int err;
> >
> > bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
> > if (!bridge)
> > return -ENOMEM;
> >
> > /* setup bridge (return -EPROBE_DEFER if necessary, ...) */
> >
> > /* register bridge with DRM */
> > drm_bridge_init(&bridge->base);
> > bridge->base.dev = dev;
> > bridge->base.funcs = &foo_bridge_funcs;
> >
> > err = drm_bridge_add(&bridge->base);
> > if (err < 0)
> > return err;
> >
> > dev_set_drvdata(dev, bridge);
> > ...
> > }
> >
> > drm_bridge_add() would add the bridge to a global list of bridge devices
> > which drm_bridge_get()/of_drm_find_bridge() can use to find the one that
> > it needs. The above has the big advantage that
> > it'sdev->mode_config.bridge_list completely
> > independent of the underlying bus that the bridge is on. It could be I2C
> > or SPI, platform device or PCI device.
> >
> > Thierry
> Ok. This is all about making the bridge driver confine to the driver model.
> But, I would need a drm_device pointer to register the bridge to DRM core.
> How do I get it?
Once you've obtained a reference to the DRM bridge from your driver you
should carry it around until you've set up the DRM device. Then you can
hook it up, which possibly requires a new function (since it's what the
drm_bridge_init() used to do).
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/0e4bd46b/attachment-0001.sig>
[Bug 81476] three monitors on two radeon cards works with some layouts not others
https://bugs.freedesktop.org/show_bug.cgi?id=81476 --- Comment #18 from Alex Deucher --- (In reply to comment #17) > > Phoronix suggests they don't all work well with open source drivers, those > that do work often don't do well multi-monitor, and most don't do triple DVI > output unless one buys one or more active adapters. Displays should generally work fine. Note that Evergreen, NI, and SI parts only have two non-DP PLLs so they are limited to two independent non-DP clocks. So what this means is that you can only use two non-DP monitors with independent timings. If you want to use more than two non-DP monitors, you have to use the exact same pixel clocks on several of the monitors so you don't use more then two different clocks. E.g., if all three of your monitors are identical use the same modeline and pixel clock, you'd only need one PLL so you should be fine. > > I don't want to pick a card that becomes obsolete and where the driver > writers have moved on and aren't interested in fixing the old bugs (e.g. the > above bug in the radeon driver). TBH, I think it's a hw limitation on r5xx hardware that's not properly handled. Unfortunately, X didn't support multi-card xrandr support when r5xx was still actively being developed. At that time, the only way to support multi-card multi-head was zaphod mode which uses separate buffers for each display so you never hit the hw limits. > > Are the open source drivers for the evergreen cards (and newer) under active > development? Yes. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/dea2c565/attachment.html>
[PATCH V5 11/12] Documentation: Add DT bindings for ps8622/ps8625 bridge driver
Hi Thierry,
On Mon, Jul 21, 2014 at 12:36 PM, Thierry Reding
wrote:
> On Fri, Jul 18, 2014 at 02:13:57AM +0530, Ajay Kumar wrote:
>> From: Vincent Palatin
>>
>> Add DT binding documentation for ps8622/ps8625 bridge driver.
>>
>> Signed-off-by: Vincent Palatin
>> Signed-off-by: Ajay Kumar
>> ---
>> .../devicetree/bindings/drm/bridge/ps8622.txt | 21
>>
>
> This is the wrong directory. Bindings are supposed to be OS agnostic,
> but DRM is (mostly) Linux-specific. video/bridge would be a better
> subdirectory for this. Somebody really ought to be moving out the
> existing bindings in the drm subdirectory to a more proper location.
Ok. I will move them out into video/bridge.
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>>
>> diff --git a/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>> b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>> new file mode 100644
>> index 000..1d154ac
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/drm/bridge/ps8622.txt
>> @@ -0,0 +1,21 @@
>> +ps8622-bridge bindings
>> +
>> +Required properties:
>> + - compatible: "parade,ps8622" or "parade,ps8625"
>
> Documentation/devicetree/bindings/vendor-prefixes.txt doesn't contain an
> entry for parade yet.
I will fix this.
>> + - reg: first i2c address of the bridge
>> + - sleep-gpios: OF device-tree gpio specification
>> + - reset-gpios: OF device-tree gpio specification
>> +
>> +Optional properties:
>> + - lane-count: number of DP lanes to use
>> + - use-external-pwm: backlight will be controlled by an external PWM
>
> In case of an external backlight, don't you still need a way to control
> it? Perhaps instead of using this boolean property you should make this
> take a phandle to the real backlight? Like so:
>
> backlight {
> compatible = "pwm-backlight";
> ...
> }
>
> bridge at 48 {
> compatible = "parade,ps8622";
> ...
> backlight = <&/backlight>;
> }
>
> Then you can simply look up the backlight device when that property
> exists and instantiate the built-in backlight otherwise.
Thanks for the pointer, this approach seems perfect!
>> +
>> +Example:
>> + ps8622-bridge at 48 {
>> + compatible = "parade,ps8622";
>> + reg = <0x48>;
>> + sleep-gpios = <&gpc3 6 1 0 0>;
>> + reset-gpios = <&gpc3 1 1 0 0>;
>> + display-timings = <&lcd_display_timings>;
>
> Why is this specifying display-timings? It's not mentioned in the
> binding above and I don't see why the bridge would need to provide
> one since it's really a property of the panel.
Ahh. I was trying to copy bindings from previous patchset and I messed it up.
Will fix it.
Ajay
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 15:54:12 +0200 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 03:47:52PM +0200, Laurent Pinchart wrote: > > Hi Boris, > > > > On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > > > > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > > > >> On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > > >>> On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > > > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > > >> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > > >> > > > >> [snip] > > > >> > > > >>> The new drm_display_info structure should look like this [2] > > > >>> (except that color_formats and bpc have not be removed yet), and > > > >>> [1] is just here to show how the video_bus_format enum would look > > > >>> like. > > > >>> > > > >>> [1] http://code.bulix.org/rfd0yx-86557 > > > >>> [2] http://code.bulix.org/7n03b4-86556 > > > >> > > > >> Quoting from your paste: > > > >>+ const enum video_bus_format *bus_formats; > > > >>+ int nbus_formats; > > > >> > > > >> Do we really need more than one? > > > > > > > > We do if we want to replace the color_formats and bpc fields. > > > > > > Yes, that's what I was about to answer :-). > > > >>> > > > >>> Maybe we don't need to replace color_formats and bpc field > > > >>> immediately. That could be done in a follow-up patch. > > > >> > > > >> We don't need to replace them right now, but we should at least agree > > > >> on > > > >> how to replace them. Introducing a new field that would need to be > > > >> replaced in the near future when removing color_formats and bpc would > > > >> be a waste of time. > > > > > > > > Sure. One of the problems I see with replacing color_formats and bpc > > > > with the above is that some of the bits within color_formats are set > > > > when the EDID is parsed. That implies that if they are replaced with > > > > an array of formats, the array would need to be reallocated during EDID > > > > parsing. That sounds like ugliness. > > > > > > > > But if you can find a nice way to make it work that'd be great. > > > > > > How about using a list instead of an array ? > > > This way we can add elements to this list when parsing the EDID. > > > > > > Or we can just define a maximum size for the bus_formats array when > > > retrieving this info from EDID. If I'm correct we have at most 18 bus > > > formats: > > > - 3 color formats: > > >* RGB 4:4:4 > > >* YCbCr 4:4:4 > > >* YCbCr 4:4:2 > > > - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color > > > > bpc isn't a bitmask, so EDID supports up to three formats only. > > > > The color_formats field is computed in the drm_add_display_info() function. > > You could easily turn it into a local variable and allocate and fill the > > formats array at the end of the function. > > But you also need to be careful to keep whatever formats the driver > might have set explicitly. Okay, in this case, using a list is a better idea, don't you think ? -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[Bug 80851] New: Radeon[Trinity HD 7520G] Resume Results in Blank Screen, no backlight
https://bugzilla.kernel.org/show_bug.cgi?id=80851 Bug ID: 80851 Summary: Radeon[Trinity HD 7520G] Resume Results in Blank Screen, no backlight Product: Drivers Version: 2.5 Kernel Version: 3.15.6 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-dri at kernel-bugs.osdl.org Reporter: swoorupj at gmail.com Regression: No Created attachment 143781 --> https://bugzilla.kernel.org/attachment.cgi?id=143781&action=edit dmesg log Hi, When using the radeon DRM module in kernel, putting the system back to sleep and resuming always results in blank screen. It only works if the radeon kernel module is disabled. My System Specs: Asus K55N laptop Processory: AMD A6-4400M APU Graphics: Radeon HD 7520G With windows 7/8, resuming just simply works fine though Regards, Swoorup -- You are receiving this mail because: You are watching the assignee of the bug.
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 15:47:52 +0200 Laurent Pinchart wrote: > Hi Boris, > > On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > > > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > > >> On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > >>> On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > >> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > >> > > >> [snip] > > >> > > >>> The new drm_display_info structure should look like this [2] > > >>> (except that color_formats and bpc have not be removed yet), and > > >>> [1] is just here to show how the video_bus_format enum would look > > >>> like. > > >>> > > >>> [1] http://code.bulix.org/rfd0yx-86557 > > >>> [2] http://code.bulix.org/7n03b4-86556 > > >> > > >> Quoting from your paste: > > >> + const enum video_bus_format *bus_formats; > > >> + int nbus_formats; > > >> > > >> Do we really need more than one? > > > > > > We do if we want to replace the color_formats and bpc fields. > > > > Yes, that's what I was about to answer :-). > > >>> > > >>> Maybe we don't need to replace color_formats and bpc field > > >>> immediately. That could be done in a follow-up patch. > > >> > > >> We don't need to replace them right now, but we should at least agree on > > >> how to replace them. Introducing a new field that would need to be > > >> replaced in the near future when removing color_formats and bpc would > > >> be a waste of time. > > > > > > Sure. One of the problems I see with replacing color_formats and bpc > > > with the above is that some of the bits within color_formats are set > > > when the EDID is parsed. That implies that if they are replaced with > > > an array of formats, the array would need to be reallocated during EDID > > > parsing. That sounds like ugliness. > > > > > > But if you can find a nice way to make it work that'd be great. > > > > How about using a list instead of an array ? > > This way we can add elements to this list when parsing the EDID. > > > > Or we can just define a maximum size for the bus_formats array when > > retrieving this info from EDID. If I'm correct we have at most 18 bus > > formats: > > - 3 color formats: > >* RGB 4:4:4 > >* YCbCr 4:4:4 > >* YCbCr 4:4:2 > > - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color > > bpc isn't a bitmask, so EDID supports up to three formats only. Yes, bpc only contains a single value for now, and it fits the DRM_EDID_DIGITAL_DEPTH field [1] (an enum defining the supported pixel depth). ITOH, DRM_EDID_HDMI_DC_XX [2] (which are referenced in the new drm_assign_hdmi_deep_color_info function) are just bitmasks and thus a display might support several color depth. As a result, I wonder if we shouldn't start supporting several color depths (as we do for color formats). [1]http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_edid.c#L3436 [2]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_edid.c?id=refs/tags/v3.16-rc6#n3440 -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[Bug 81476] three monitors on two radeon cards works with some layouts not others
https://bugs.freedesktop.org/show_bug.cgi?id=81476 --- Comment #17 from Ian! D. Allen --- (In reply to comment #16) > All evergreen (hd 5000 series) and newer dGPUs can support up to 6 > monitors depending on the board. Phoronix suggests they don't all work well with open source drivers, those that do work often don't do well multi-monitor, and most don't do triple DVI output unless one buys one or more active adapters. I don't want to pick a card that becomes obsolete and where the driver writers have moved on and aren't interested in fixing the old bugs (e.g. the above bug in the radeon driver). Are the open source drivers for the evergreen cards (and newer) under active development? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/0495e0fc/attachment.html>
[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface
On 07/17/2014 11:01 AM, YoungJun Cho wrote:
> To support LCD I80 interface, the DSI host should register
> TE interrupt handler from the TE GPIO of attached panel.
> So the panel generates a tearing effect synchronization signal
> then the DSI host calls the CRTC device manager to trigger
> to transfer video image.
>
This whole patch seems to be a hack.
I think it is not a good idea to parse property of one device by another
device's driver.
Especially here TE GPIO belongs to the panel. The panel driver should
know how to configure it and how to use it or not. The panel driver will
generate TE signal based on this GPIO, DSI/BTA mechanism or any other
mechanism provided by the panel.
TE signal should be delivered to the display controller. The only role
of DSIM here is that it is between the panel and the display controller
so it should be used to route this signal to DC.
According to specs TE signal should/can be generated by DBI and DSI
command mode panels, so its handling should be more generic, not Exynos
specific.
Regards
Andrzej
> Signed-off-by: YoungJun Cho
> Acked-by: Inki Dae
> Acked-by: Kyungmin Park
> ---
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 95
> -
> 1 file changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 58bfb2a..4997bfe 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -16,7 +16,9 @@
> #include
>
> #include
> +#include
> #include
> +#include
> #include
> #include
> #include
> @@ -24,6 +26,7 @@
> #include
> #include
>
> +#include "exynos_drm_crtc.h"
> #include "exynos_drm_drv.h"
>
> /* returns true iff both arguments logically differs */
> @@ -247,6 +250,7 @@ struct exynos_dsi {
> struct clk *bus_clk;
> struct regulator_bulk_data supplies[2];
> int irq;
> + int te_gpio;
>
> u32 pll_clk_rate;
> u32 burst_clk_rate;
> @@ -954,17 +958,89 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
> +{
> + struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
> + struct drm_encoder *encoder = dsi->encoder;
> +
> + if (dsi->state & DSIM_STATE_ENABLED)
> + exynos_drm_crtc_te_handler(encoder->crtc);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void exynos_dsi_enable_irq(struct exynos_dsi *dsi)
> +{
> + enable_irq(dsi->irq);
> +
> + if (gpio_is_valid(dsi->te_gpio))
> + enable_irq(gpio_to_irq(dsi->te_gpio));
> +}
> +
> +static void exynos_dsi_disable_irq(struct exynos_dsi *dsi)
> +{
> + if (gpio_is_valid(dsi->te_gpio))
> + disable_irq(gpio_to_irq(dsi->te_gpio));
> +
> + disable_irq(dsi->irq);
> +}
> +
> static int exynos_dsi_init(struct exynos_dsi *dsi)
> {
> exynos_dsi_enable_clock(dsi);
> exynos_dsi_reset(dsi);
> - enable_irq(dsi->irq);
> + exynos_dsi_enable_irq(dsi);
> exynos_dsi_wait_for_reset(dsi);
> exynos_dsi_init_link(dsi);
>
> return 0;
> }
>
> +static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
> +{
> + int ret;
> +
> + dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
> + if (!gpio_is_valid(dsi->te_gpio)) {
> + dev_err(dsi->dev, "no te-gpios specified\n");
> + ret = dsi->te_gpio;
> + goto out;
> + }
> +
> + ret = gpio_request_one(dsi->te_gpio, GPIOF_IN, "te_gpio");
> + if (ret) {
> + dev_err(dsi->dev, "gpio request failed with %d\n", ret);
> + goto out;
> + }
> +
> + /*
> + * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
> + * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
> + * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
> + * called by drm_panel_init() before panel is attached.
> + */
> + ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
> + exynos_dsi_te_irq_handler, NULL,
> + IRQF_TRIGGER_RISING, "TE", dsi);
> + if (ret) {
> + dev_err(dsi->dev, "request interrupt failed with %d\n", ret);
> + gpio_free(dsi->te_gpio);
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
> +{
> + if (gpio_is_valid(dsi->te_gpio)) {
> + free_irq(gpio_to_irq(dsi->te_gpio), dsi);
> + gpio_free(dsi->te_gpio);
> + dsi->te_gpio = -ENOENT;
> + }
> +}
> +
> static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> struct mipi_dsi_device *device)
> {
> @@ -978,6 +1054,16 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host
> *host,
> if
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 03:47:52PM +0200, Laurent Pinchart wrote: > Hi Boris, > > On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > > > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > > >> On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > >>> On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > >>>> On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > >>>>> On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > >>>>>> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > >> > > >> [snip] > > >> > > >>>>>>> The new drm_display_info structure should look like this [2] > > >>>>>>> (except that color_formats and bpc have not be removed yet), and > > >>>>>>> [1] is just here to show how the video_bus_format enum would look > > >>>>>>> like. > > >>>>>>> > > >>>>>>> [1] http://code.bulix.org/rfd0yx-86557 > > >>>>>>> [2] http://code.bulix.org/7n03b4-86556 > > >>>>>> > > >>>>>> Quoting from your paste: > > >>>>>> + const enum video_bus_format *bus_formats; > > >>>>>> + int nbus_formats; > > >>>>>> > > >>>>>> Do we really need more than one? > > >>>>> > > >>>>> We do if we want to replace the color_formats and bpc fields. > > >>>> > > >>>> Yes, that's what I was about to answer :-). > > >>> > > >>> Maybe we don't need to replace color_formats and bpc field > > >>> immediately. That could be done in a follow-up patch. > > >> > > >> We don't need to replace them right now, but we should at least agree on > > >> how to replace them. Introducing a new field that would need to be > > >> replaced in the near future when removing color_formats and bpc would > > >> be a waste of time. > > > > > > Sure. One of the problems I see with replacing color_formats and bpc > > > with the above is that some of the bits within color_formats are set > > > when the EDID is parsed. That implies that if they are replaced with > > > an array of formats, the array would need to be reallocated during EDID > > > parsing. That sounds like ugliness. > > > > > > But if you can find a nice way to make it work that'd be great. > > > > How about using a list instead of an array ? > > This way we can add elements to this list when parsing the EDID. > > > > Or we can just define a maximum size for the bus_formats array when > > retrieving this info from EDID. If I'm correct we have at most 18 bus > > formats: > > - 3 color formats: > >* RGB 4:4:4 > >* YCbCr 4:4:4 > >* YCbCr 4:4:2 > > - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color > > bpc isn't a bitmask, so EDID supports up to three formats only. > > The color_formats field is computed in the drm_add_display_info() function. > You could easily turn it into a local variable and allocate and fill the > formats array at the end of the function. But you also need to be careful to keep whatever formats the driver might have set explicitly. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/2013db14/attachment.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Boris, On Monday 21 July 2014 15:43:13 Boris BREZILLON wrote: > On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > >> On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > >>> On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > >> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > >> > >> [snip] > >> > >>> The new drm_display_info structure should look like this [2] > >>> (except that color_formats and bpc have not be removed yet), and > >>> [1] is just here to show how the video_bus_format enum would look > >>> like. > >>> > >>> [1] http://code.bulix.org/rfd0yx-86557 > >>> [2] http://code.bulix.org/7n03b4-86556 > >> > >> Quoting from your paste: > >>+ const enum video_bus_format *bus_formats; > >>+ int nbus_formats; > >> > >> Do we really need more than one? > > > > We do if we want to replace the color_formats and bpc fields. > > Yes, that's what I was about to answer :-). > >>> > >>> Maybe we don't need to replace color_formats and bpc field > >>> immediately. That could be done in a follow-up patch. > >> > >> We don't need to replace them right now, but we should at least agree on > >> how to replace them. Introducing a new field that would need to be > >> replaced in the near future when removing color_formats and bpc would > >> be a waste of time. > > > > Sure. One of the problems I see with replacing color_formats and bpc > > with the above is that some of the bits within color_formats are set > > when the EDID is parsed. That implies that if they are replaced with > > an array of formats, the array would need to be reallocated during EDID > > parsing. That sounds like ugliness. > > > > But if you can find a nice way to make it work that'd be great. > > How about using a list instead of an array ? > This way we can add elements to this list when parsing the EDID. > > Or we can just define a maximum size for the bus_formats array when > retrieving this info from EDID. If I'm correct we have at most 18 bus > formats: > - 3 color formats: >* RGB 4:4:4 >* YCbCr 4:4:4 >* YCbCr 4:4:2 > - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color bpc isn't a bitmask, so EDID supports up to three formats only. The color_formats field is computed in the drm_add_display_info() function. You could easily turn it into a local variable and allocate and fill the formats array at the end of the function. -- Regards, Laurent Pinchart
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 15:30:35 +0200 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > > Hi Thierry, > > > > On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > > On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > > > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > > >> On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > > >>> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > > > [snip] > > > > > The new drm_display_info structure should look like this [2] (except > > > that color_formats and bpc have not be removed yet), and [1] is just > > > here to show how the video_bus_format enum would look like. > > > > > > [1] http://code.bulix.org/rfd0yx-86557 > > > [2] http://code.bulix.org/7n03b4-86556 > > > >>> > > > >>> Quoting from your paste: > > > >>> + const enum video_bus_format *bus_formats; > > > >>> + int nbus_formats; > > > >>> > > > >>> Do we really need more than one? > > > >> > > > >> We do if we want to replace the color_formats and bpc fields. > > > > > > > > Yes, that's what I was about to answer :-). > > > > > > Maybe we don't need to replace color_formats and bpc field immediately. > > > That could be done in a follow-up patch. > > > > We don't need to replace them right now, but we should at least agree on > > how > > to replace them. Introducing a new field that would need to be replaced in > > the > > near future when removing color_formats and bpc would be a waste of time. > > Sure. One of the problems I see with replacing color_formats and bpc > with the above is that some of the bits within color_formats are set > when the EDID is parsed. That implies that if they are replaced with > an array of formats, the array would need to be reallocated during EDID > parsing. That sounds like ugliness. > > But if you can find a nice way to make it work that'd be great. How about using a list instead of an array ? This way we can add elements to this list when parsing the EDID. Or we can just define a maximum size for the bus_formats array when retrieving this info from EDID. If I'm correct we have at most 18 bus formats: - 3 color formats: * RGB 4:4:4 * YCbCr 4:4:4 * YCbCr 4:4:2 - 6 color depths: 6, 8, 10, 12, 14 and 16 bits per color -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH v2 00/25] AMDKFD kernel driver
Am 21.07.2014 14:36, schrieb Oded Gabbay: > On 20/07/14 20:46, Jerome Glisse wrote: >> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >>> Forgot to cc mailing list on cover letter. Sorry. >>> >>> As a continuation to the existing discussion, here is a v2 patch series >>> restructured with a cleaner history and no >>> totally-different-early-versions >>> of the code. >>> >>> Instead of 83 patches, there are now a total of 25 patches, where 5 >>> of them >>> are modifications to radeon driver and 18 of them include only >>> amdkfd code. >>> There is no code going away or even modified between patches, only >>> added. >>> >>> The driver was renamed from radeon_kfd to amdkfd and moved to reside >>> under >>> drm/radeon/amdkfd. This move was done to emphasize the fact that >>> this driver >>> is an AMD-only driver at this point. Having said that, we do foresee a >>> generic hsa framework being implemented in the future and in that >>> case, we >>> will adjust amdkfd to work within that framework. >>> >>> As the amdkfd driver should support multiple AMD gfx drivers, we >>> want to >>> keep it as a seperate driver from radeon. Therefore, the amdkfd code is >>> contained in its own folder. The amdkfd folder was put under the radeon >>> folder because the only AMD gfx driver in the Linux kernel at this >>> point >>> is the radeon driver. Having said that, we will probably need to >>> move it >>> (maybe to be directly under drm) after we integrate with additional >>> AMD gfx >>> drivers. >>> >>> For people who like to review using git, the v2 patch set is located >>> at: >>> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >>> >>> Written by Oded Gabbayh >> >> So quick comments before i finish going over all patches. There is many >> things that need more documentation espacialy as of right now there is >> no userspace i can go look at. > So quick comments on some of your questions but first of all, thanks > for the time you dedicated to review the code. >> >> There few show stopper, biggest one is gpu memory pinning this is a big >> no, that would need serious arguments for any hope of convincing me on >> that side. > We only do gpu memory pinning for kernel objects. There are no > userspace objects that are pinned on the gpu memory in our driver. If > that is the case, is it still a show stopper ? > > The kernel objects are: > - pipelines (4 per device) > - mqd per hiq (only 1 per device) > - mqd per userspace queue. On KV, we support up to 1K queues per > process, for a total of 512K queues. Each mqd is 151 bytes, but the > allocation is done in 256 alignment. So total *possible* memory is 128MB > - kernel queue (only 1 per device) > - fence address for kernel queue > - runlists for the CP (1 or 2 per device) The main questions here are if it's avoid able to pin down the memory and if the memory is pinned down at driver load, by request from userspace or by anything else. As far as I can see only the "mqd per userspace queue" might be a bit questionable, everything else sounds reasonable. Christian. >> >> It might be better to add a drivers/gpu/drm/amd directory and add common >> stuff there. >> >> Given that this is not intended to be final HSA api AFAICT then i would >> say this far better to avoid the whole kfd module and add ioctl to >> radeon. >> This would avoid crazy communication btw radeon and kfd. >> >> The whole aperture business needs some serious explanation. Especialy as >> you want to use userspace address there is nothing to prevent userspace >> program from allocating things at address you reserve for lds, scratch, >> ... only sane way would be to move those lds, scratch inside the virtual >> address reserved for kernel (see kernel memory map). >> >> The whole business of locking performance counter for exclusive per >> process >> access is a big NO. Which leads me to the questionable usefullness of >> user >> space command ring. > That's like saying: "Which leads me to the questionable usefulness of > HSA". I find it analogous to a situation where a network maintainer > nacking a driver for a network card, which is slower than a different > network card. Doesn't seem reasonable this situation is would happen. > He would still put both the drivers in the kernel because people want > to use the H/W and its features. So, I don't think this is a valid > reason to NACK the driver. > >> I only see issues with that. First and foremost i would >> need to see solid figures that kernel ioctl or syscall has a higher an >> overhead that is measurable in any meaning full way against a simple >> function call. I know the userspace command ring is a big marketing >> features >> that please ignorant userspace programmer. But really this only >> brings issues >> and for absolutely not upside afaict. > Really ? You think that doing a context switch to kernel space, with > all its overhead, is _not_ more expansive than just calling a function >
[PATCH v2 00/25] AMDKFD kernel driver
On 20/07/14 20:46, Jerome Glisse wrote: > On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >> Forgot to cc mailing list on cover letter. Sorry. >> >> As a continuation to the existing discussion, here is a v2 patch series >> restructured with a cleaner history and no totally-different-early-versions >> of the code. >> >> Instead of 83 patches, there are now a total of 25 patches, where 5 of them >> are modifications to radeon driver and 18 of them include only amdkfd code. >> There is no code going away or even modified between patches, only added. >> >> The driver was renamed from radeon_kfd to amdkfd and moved to reside under >> drm/radeon/amdkfd. This move was done to emphasize the fact that this driver >> is an AMD-only driver at this point. Having said that, we do foresee a >> generic hsa framework being implemented in the future and in that case, we >> will adjust amdkfd to work within that framework. >> >> As the amdkfd driver should support multiple AMD gfx drivers, we want to >> keep it as a seperate driver from radeon. Therefore, the amdkfd code is >> contained in its own folder. The amdkfd folder was put under the radeon >> folder because the only AMD gfx driver in the Linux kernel at this point >> is the radeon driver. Having said that, we will probably need to move it >> (maybe to be directly under drm) after we integrate with additional AMD gfx >> drivers. >> >> For people who like to review using git, the v2 patch set is located at: >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >> >> Written by Oded Gabbayh > > So quick comments before i finish going over all patches. There is many > things that need more documentation espacialy as of right now there is > no userspace i can go look at. So quick comments on some of your questions but first of all, thanks for the time you dedicated to review the code. > > There few show stopper, biggest one is gpu memory pinning this is a big > no, that would need serious arguments for any hope of convincing me on > that side. We only do gpu memory pinning for kernel objects. There are no userspace objects that are pinned on the gpu memory in our driver. If that is the case, is it still a show stopper ? The kernel objects are: - pipelines (4 per device) - mqd per hiq (only 1 per device) - mqd per userspace queue. On KV, we support up to 1K queues per process, for a total of 512K queues. Each mqd is 151 bytes, but the allocation is done in 256 alignment. So total *possible* memory is 128MB - kernel queue (only 1 per device) - fence address for kernel queue - runlists for the CP (1 or 2 per device) > > It might be better to add a drivers/gpu/drm/amd directory and add common > stuff there. > > Given that this is not intended to be final HSA api AFAICT then i would > say this far better to avoid the whole kfd module and add ioctl to radeon. > This would avoid crazy communication btw radeon and kfd. > > The whole aperture business needs some serious explanation. Especialy as > you want to use userspace address there is nothing to prevent userspace > program from allocating things at address you reserve for lds, scratch, > ... only sane way would be to move those lds, scratch inside the virtual > address reserved for kernel (see kernel memory map). > > The whole business of locking performance counter for exclusive per process > access is a big NO. Which leads me to the questionable usefullness of user > space command ring. That's like saying: "Which leads me to the questionable usefulness of HSA". I find it analogous to a situation where a network maintainer nacking a driver for a network card, which is slower than a different network card. Doesn't seem reasonable this situation is would happen. He would still put both the drivers in the kernel because people want to use the H/W and its features. So, I don't think this is a valid reason to NACK the driver. > I only see issues with that. First and foremost i would > need to see solid figures that kernel ioctl or syscall has a higher an > overhead that is measurable in any meaning full way against a simple > function call. I know the userspace command ring is a big marketing features > that please ignorant userspace programmer. But really this only brings issues > and for absolutely not upside afaict. Really ? You think that doing a context switch to kernel space, with all its overhead, is _not_ more expansive than just calling a function in userspace which only puts a buffer on a ring and writes a doorbell ? > > So i would rather see a very simple ioctl that write the doorbell and might > do more than that in case of ring/queue overcommit where it would first have > to wait for a free ring/queue to schedule stuff. This would also allow sane > implementation of things like performance counter that could be acquire by > kernel for duration of a job submitted by userspace. While still not optimal > this would be better that userspace locking. > > > I might have more
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 03:26:12PM +0200, Laurent Pinchart wrote: > Hi Thierry, > > On Monday 21 July 2014 14:56:26 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 02:33:21PM +0200, Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 14:15:16 +0200 Thierry Reding wrote: > > >> On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote: > > >>> On Tue, 15 Jul 2014 12:31:37 +0200 Thierry Reding wrote: > > >>>> On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote: > > >>>>> On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding wrote: > > >>>>>> On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote: > > [snip] > > > >>>>>>> + - atmel,panel: Should contain a phandle with 2 parameters. > > >>>>>>> + The first cell is a phandle to a DRM panel device > > >>>>>>> + The second cell encodes the RGB mode, which can take the > > >>>>>>> following values: + * 0: RGB444 > > >>>>>>> + * 1: RGB565 > > >>>>>>> + * 2: RGB666 > > >>>>>>> + * 3: RGB888 > > >>>>>> > > >>>>>> These are properties of the panel and should be obtained from > > >>>>>> the panel directly rather than an additional cell in this > > >>>>>> specifier. > > >>>>> > > >>>>> Okay. > > >>>>> What's the preferred way of doing this ? > > >>>>> What about defining an rgb-mode property in the panel node. > > [snip] > > > >>>> Also, like Laurent said, this shouldn't go into the device tree, > > >>>> since it's already implied by the panel's compatible value, so we'd > > >>>> be duplicating information. > > >>> > > >>> Again, this is not necessarily true (depending on your board design). > > >>> One can decide to connect an RGB888 panel on an RGB666 bus and connect > > >>> the missing pins to ground. > > >> > > >> I think in that case the board design itself could be considered as an > > >> RGB888 to RGB666 bridge, and I think that's what the device tree should > > >> be describing rather than a panel with a variable number of input > > >> formats. > > > > > > So, you're suggesting to add an RGB to RGB drm_bridge driver (and > > > the appropriate DT bindings) to handle this case, right ? > > > > Yes, exactly. > > Wouldn't it be possible to implement RGB666 -> RGB888 support in a less > complex way ? A standalone driver to describe signal routing seems like an > overly complex solution to me. I would prefer making the routing a properly > of > the link instead of a separate device. I don't think the above is overly complex. After all the panel expects RGB888, so it makes no sense to make it "configurable" to anything else. Similarly if the encoder or bridge provides RGB666 then that's a fixed function, too. So to represent this combination accurately you'll need some form of translation entity inbetween, and it may just as well be a bridge than something custom for that particular link. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/59b85ab0/attachment.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 03:22:05PM +0200, Laurent Pinchart wrote: > Hi Thierry, > > On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > > >> On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > >>> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > [snip] > > > >>>> The new drm_display_info structure should look like this [2] (except > > >>>> that color_formats and bpc have not be removed yet), and [1] is just > > >>>> here to show how the video_bus_format enum would look like. > > >>>> > > >>>> [1] http://code.bulix.org/rfd0yx-86557 > > >>>> [2] http://code.bulix.org/7n03b4-86556 > > >>> > > >>> Quoting from your paste: > > >>> + const enum video_bus_format *bus_formats; > > >>> + int nbus_formats; > > >>> > > >>> Do we really need more than one? > > >> > > >> We do if we want to replace the color_formats and bpc fields. > > > > > > Yes, that's what I was about to answer :-). > > > > Maybe we don't need to replace color_formats and bpc field immediately. > > That could be done in a follow-up patch. > > We don't need to replace them right now, but we should at least agree on how > to replace them. Introducing a new field that would need to be replaced in > the > near future when removing color_formats and bpc would be a waste of time. Sure. One of the problems I see with replacing color_formats and bpc with the above is that some of the bits within color_formats are set when the EDID is parsed. That implies that if they are replaced with an array of formats, the array would need to be reallocated during EDID parsing. That sounds like ugliness. But if you can find a nice way to make it work that'd be great. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/46167207/attachment.sig>
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 10:23:43PM +0300, Oded Gabbay wrote: > On 21/07/14 21:59, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 09:36:44PM +0300, Oded Gabbay wrote: > >> On 21/07/14 21:14, Jerome Glisse wrote: > >>> On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: > On 21/07/14 18:54, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: > >> On 21/07/14 16:39, Christian K?nig wrote: > >>> Am 21.07.2014 14:36, schrieb Oded Gabbay: > On 20/07/14 20:46, Jerome Glisse wrote: > > On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > >> Forgot to cc mailing list on cover letter. Sorry. > >> > >> As a continuation to the existing discussion, here is a v2 patch > >> series > >> restructured with a cleaner history and no > >> totally-different-early-versions > >> of the code. > >> > >> Instead of 83 patches, there are now a total of 25 patches, where > >> 5 of them > >> are modifications to radeon driver and 18 of them include only > >> amdkfd code. > >> There is no code going away or even modified between patches, only > >> added. > >> > >> The driver was renamed from radeon_kfd to amdkfd and moved to > >> reside under > >> drm/radeon/amdkfd. This move was done to emphasize the fact that > >> this driver > >> is an AMD-only driver at this point. Having said that, we do > >> foresee a > >> generic hsa framework being implemented in the future and in that > >> case, we > >> will adjust amdkfd to work within that framework. > >> > >> As the amdkfd driver should support multiple AMD gfx drivers, we > >> want to > >> keep it as a seperate driver from radeon. Therefore, the amdkfd > >> code is > >> contained in its own folder. The amdkfd folder was put under the > >> radeon > >> folder because the only AMD gfx driver in the Linux kernel at this > >> point > >> is the radeon driver. Having said that, we will probably need to > >> move it > >> (maybe to be directly under drm) after we integrate with > >> additional AMD gfx > >> drivers. > >> > >> For people who like to review using git, the v2 patch set is > >> located at: > >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > >> > >> Written by Oded Gabbayh > > > > So quick comments before i finish going over all patches. There is > > many > > things that need more documentation espacialy as of right now there > > is > > no userspace i can go look at. > So quick comments on some of your questions but first of all, thanks > for the > time you dedicated to review the code. > > > > There few show stopper, biggest one is gpu memory pinning this is a > > big > > no, that would need serious arguments for any hope of convincing me > > on > > that side. > We only do gpu memory pinning for kernel objects. There are no > userspace > objects that are pinned on the gpu memory in our driver. If that is > the case, > is it still a show stopper ? > > The kernel objects are: > - pipelines (4 per device) > - mqd per hiq (only 1 per device) > - mqd per userspace queue. On KV, we support up to 1K queues per > process, for > a total of 512K queues. Each mqd is 151 bytes, but the allocation is > done in > 256 alignment. So total *possible* memory is 128MB > - kernel queue (only 1 per device) > - fence address for kernel queue > - runlists for the CP (1 or 2 per device) > >>> > >>> The main questions here are if it's avoid able to pin down the memory > >>> and if the > >>> memory is pinned down at driver load, by request from userspace or by > >>> anything > >>> else. > >>> > >>> As far as I can see only the "mqd per userspace queue" might be a bit > >>> questionable, everything else sounds reasonable. > >>> > >>> Christian. > >> > >> Most of the pin downs are done on device initialization. > >> The "mqd per userspace" is done per userspace queue creation. However, > >> as I > >> said, it has an upper limit of 128MB on KV, and considering the 2G > >> local > >> memory, I think it is OK. > >> The runlists are also done on userspace queue creation/deletion, but > >> we only > >> have 1 or 2 runlists per device, so it is not that bad. > > > > 2G local memory ? You can not assume anything on userside configuration > > some > > one might build
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Thierry, On Monday 21 July 2014 14:56:26 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 02:33:21PM +0200, Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 14:15:16 +0200 Thierry Reding wrote: > >> On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote: > >>> On Tue, 15 Jul 2014 12:31:37 +0200 Thierry Reding wrote: > >>>> On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote: > >>>>> On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding wrote: > >>>>>> On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote: [snip] > >>>>>>> + - atmel,panel: Should contain a phandle with 2 parameters. > >>>>>>> + The first cell is a phandle to a DRM panel device > >>>>>>> + The second cell encodes the RGB mode, which can take the > >>>>>>> following values: + * 0: RGB444 > >>>>>>> + * 1: RGB565 > >>>>>>> + * 2: RGB666 > >>>>>>> + * 3: RGB888 > >>>>>> > >>>>>> These are properties of the panel and should be obtained from > >>>>>> the panel directly rather than an additional cell in this > >>>>>> specifier. > >>>>> > >>>>> Okay. > >>>>> What's the preferred way of doing this ? > >>>>> What about defining an rgb-mode property in the panel node. [snip] > >>>> Also, like Laurent said, this shouldn't go into the device tree, > >>>> since it's already implied by the panel's compatible value, so we'd > >>>> be duplicating information. > >>> > >>> Again, this is not necessarily true (depending on your board design). > >>> One can decide to connect an RGB888 panel on an RGB666 bus and connect > >>> the missing pins to ground. > >> > >> I think in that case the board design itself could be considered as an > >> RGB888 to RGB666 bridge, and I think that's what the device tree should > >> be describing rather than a panel with a variable number of input > >> formats. > > > > So, you're suggesting to add an RGB to RGB drm_bridge driver (and > > the appropriate DT bindings) to handle this case, right ? > > Yes, exactly. Wouldn't it be possible to implement RGB666 -> RGB888 support in a less complex way ? A standalone driver to describe signal routing seems like an overly complex solution to me. I would prefer making the routing a properly of the link instead of a separate device. -- Regards, Laurent Pinchart -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: This is a digitally signed message part. URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/3e8f6277/attachment-0001.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Thierry, On Monday 21 July 2014 14:55:23 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > >> On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > >>> On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: [snip] > >>>> The new drm_display_info structure should look like this [2] (except > >>>> that color_formats and bpc have not be removed yet), and [1] is just > >>>> here to show how the video_bus_format enum would look like. > >>>> > >>>> [1] http://code.bulix.org/rfd0yx-86557 > >>>> [2] http://code.bulix.org/7n03b4-86556 > >>> > >>> Quoting from your paste: > >>> + const enum video_bus_format *bus_formats; > >>> + int nbus_formats; > >>> > >>> Do we really need more than one? > >> > >> We do if we want to replace the color_formats and bpc fields. > > > > Yes, that's what I was about to answer :-). > > Maybe we don't need to replace color_formats and bpc field immediately. > That could be done in a follow-up patch. We don't need to replace them right now, but we should at least agree on how to replace them. Introducing a new field that would need to be replaced in the near future when removing color_formats and bpc would be a waste of time. -- Regards, Laurent Pinchart -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: This is a digitally signed message part. URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/72642444/attachment.sig>
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 09:41:29PM +0300, Oded Gabbay wrote: > On 21/07/14 21:22, Daniel Vetter wrote: > > On Mon, Jul 21, 2014 at 7:28 PM, Oded Gabbay wrote: > >>> I'm not sure whether we can do the same trick with the hw scheduler. But > >>> then unpinning hw contexts will drain the pipeline anyway, so I guess we > >>> can just stop feeding the hw scheduler until it runs dry. And then unpin > >>> and evict. > >> So, I'm afraid but we can't do this for AMD Kaveri because: > > > > Well as long as you can drain the hw scheduler queue (and you can do > > that, worst case you have to unmap all the doorbells and other stuff > > to intercept further submission from userspace) you can evict stuff. > > I can't drain the hw scheduler queue, as I can't do mid-wave preemption. > Moreover, if I use the dequeue request register to preempt a queue > during a dispatch it may be that some waves (wave groups actually) of > the dispatch have not yet been created, and when I reactivate the mqd, > they should be created but are not. However, this works fine if you use > the HIQ. the CP ucode correctly saves and restores the state of an > outstanding dispatch. I don't think we have access to the state from > software at all, so it's not a bug, it is "as designed". > I think here Daniel is suggesting to unmapp the doorbell page, and track each write made by userspace to it and while unmapped wait for the gpu to drain or use some kind of fence on a special queue. Once GPU is drain we can move pinned buffer, then remap the doorbell and update it to the last value written by userspace which will resume execution to the next job. > > And if we don't want compute to be a denial of service on the display > > side of the driver we need this ability. Now if you go through an > > ioctl instead of the doorbell (I agree with Jerome here, the doorbell > > should be supported by benchmarks on linux) this gets a bit easier, > > but it's not a requirement really. > > -Daniel > > > On KV, we have the theoretical option of DOS on the display side as we > can't do a mid-wave preemption. On CZ, we won't have this problem. > > Oded
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 09:36:44PM +0300, Oded Gabbay wrote: > On 21/07/14 21:14, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: > >> On 21/07/14 18:54, Jerome Glisse wrote: > >>> On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: > On 21/07/14 16:39, Christian K?nig wrote: > > Am 21.07.2014 14:36, schrieb Oded Gabbay: > >> On 20/07/14 20:46, Jerome Glisse wrote: > >>> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > Forgot to cc mailing list on cover letter. Sorry. > > As a continuation to the existing discussion, here is a v2 patch > series > restructured with a cleaner history and no > totally-different-early-versions > of the code. > > Instead of 83 patches, there are now a total of 25 patches, where 5 > of them > are modifications to radeon driver and 18 of them include only > amdkfd code. > There is no code going away or even modified between patches, only > added. > > The driver was renamed from radeon_kfd to amdkfd and moved to reside > under > drm/radeon/amdkfd. This move was done to emphasize the fact that > this driver > is an AMD-only driver at this point. Having said that, we do foresee > a > generic hsa framework being implemented in the future and in that > case, we > will adjust amdkfd to work within that framework. > > As the amdkfd driver should support multiple AMD gfx drivers, we > want to > keep it as a seperate driver from radeon. Therefore, the amdkfd code > is > contained in its own folder. The amdkfd folder was put under the > radeon > folder because the only AMD gfx driver in the Linux kernel at this > point > is the radeon driver. Having said that, we will probably need to > move it > (maybe to be directly under drm) after we integrate with additional > AMD gfx > drivers. > > For people who like to review using git, the v2 patch set is located > at: > http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > > Written by Oded Gabbayh > >>> > >>> So quick comments before i finish going over all patches. There is > >>> many > >>> things that need more documentation espacialy as of right now there is > >>> no userspace i can go look at. > >> So quick comments on some of your questions but first of all, thanks > >> for the > >> time you dedicated to review the code. > >>> > >>> There few show stopper, biggest one is gpu memory pinning this is a > >>> big > >>> no, that would need serious arguments for any hope of convincing me on > >>> that side. > >> We only do gpu memory pinning for kernel objects. There are no > >> userspace > >> objects that are pinned on the gpu memory in our driver. If that is > >> the case, > >> is it still a show stopper ? > >> > >> The kernel objects are: > >> - pipelines (4 per device) > >> - mqd per hiq (only 1 per device) > >> - mqd per userspace queue. On KV, we support up to 1K queues per > >> process, for > >> a total of 512K queues. Each mqd is 151 bytes, but the allocation is > >> done in > >> 256 alignment. So total *possible* memory is 128MB > >> - kernel queue (only 1 per device) > >> - fence address for kernel queue > >> - runlists for the CP (1 or 2 per device) > > > > The main questions here are if it's avoid able to pin down the memory > > and if the > > memory is pinned down at driver load, by request from userspace or by > > anything > > else. > > > > As far as I can see only the "mqd per userspace queue" might be a bit > > questionable, everything else sounds reasonable. > > > > Christian. > > Most of the pin downs are done on device initialization. > The "mqd per userspace" is done per userspace queue creation. However, > as I > said, it has an upper limit of 128MB on KV, and considering the 2G local > memory, I think it is OK. > The runlists are also done on userspace queue creation/deletion, but we > only > have 1 or 2 runlists per device, so it is not that bad. > >>> > >>> 2G local memory ? You can not assume anything on userside configuration > >>> some > >>> one might build an hsa computer with 512M and still expect a functioning > >>> desktop. > >> First of all, I'm only considering Kaveri computer, not "hsa" computer. > >> Second, I would imagine we can build some protection around it, like > >> checking total local memory and limit number of queues based on some > >> percentage of that total local memory. So, if someone
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 02:33:21PM +0200, Boris BREZILLON wrote: > On Mon, 21 Jul 2014 14:15:16 +0200 > Thierry Reding wrote: > > > On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote: > > > Hi Thierry, > > > > > > Oops, I missed this reply. > > > > > > On Tue, 15 Jul 2014 12:31:37 +0200 > > > Thierry Reding wrote: > > > > > > > On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote: > > > > > On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding > > > > gmail.com> wrote: > > > > > > On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote: > > > > [...] > > > > > > > diff --git > > > > > > > a/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > > > > b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > > > [...] > > > > > > > +The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD > > > > > > > device. > > > > > > > +See Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt for > > > > > > > more details. > > > > > > > > > > > > I think it's better to refer to these using relative filenames. > > > > > > When the > > > > > > device tree bindings are moved out of the kernel tree, they may no > > > > > > longer use the same hierarchy. > > > > > > > > > > Sure. > > > > > By relative path you mean ../../mfd/atmel-hlcdc.txt or just > > > > > mfd/atmel-hlcdc.txt ? > > > > > > > > I think the former is more explicit. > > > > > > Okay. > > > > > > > > > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters. > > > > > > > + The first cell is a phandle to a DRM panel device > > > > > > > + The second cell encodes the RGB mode, which can take the > > > > > > > following values: > > > > > > > + * 0: RGB444 > > > > > > > + * 1: RGB565 > > > > > > > + * 2: RGB666 > > > > > > > + * 3: RGB888 > > > > > > > > > > > > These are properties of the panel and should be obtained from the > > > > > > panel > > > > > > directly rather than an additional cell in this specifier. > > > > > > > > > > Okay. > > > > > What's the preferred way of doing this ? > > > > > What about defining an rgb-mode property in the panel node. > > > > > > > > There's .bpc in struct drm_display_info, I suspect that it could be used > > > > for this. Alternatively, maybe we could extend the list of color formats > > > > that go into drm_display_info.color_formats? RGB444 is already covered. > > > > > > I don't think this color_formats field is intended to represent data > > > stream format going through the bus. > > > Moreover, AFAIU, RGB444 in this definition represent RGB 4:4:4 (chroma > > > subsampling rate) and not 12 bits signals (4 bits for each color). > > > > > > Anyway I'll propose a patch series adding a new field to > > > drm_display_info to encode the mediabus format (as discussed with > > > Laurent and you). > > > > > > > > > > > Also, like Laurent said, this shouldn't go into the device tree, since > > > > it's already implied by the panel's compatible value, so we'd be > > > > duplicating information. > > > > > > Again, this is not necessarily true (depending on your board design). > > > One can decide to connect an RGB888 panel on an RGB666 bus and connect > > > the missing pins to ground. > > > > I think in that case the board design itself could be considered as an > > RGB888 to RGB666 bridge, and I think that's what the device tree should > > be describing rather than a panel with a variable number of input > > formats. > > So, you're suggesting to add an RGB to RGB drm_bridge driver (and > the appropriate DT bindings) to handle this case, right ? Yes, exactly. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/1f75eb5f/attachment.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 02:34:28PM +0200, Boris BREZILLON wrote: > On Mon, 21 Jul 2014 14:16:42 +0200 > Laurent Pinchart wrote: > > > Hi Thierry, > > > > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > > On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > > > On Mon, 21 Jul 2014 11:32:55 +0200 Laurent Pinchart wrote: > > > >> On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > > > >>> On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > > > >>>> On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > > > >>>> [...] > > > >>>> > > > >>>>>>>>>> + - atmel,panel: Should contain a phandle with 2 parameters. > > > >>>>>>>>>> + The first cell is a phandle to a DRM panel device > > > >>>>>>>>>> + The second cell encodes the RGB mode, which can take the > > > >>>>>>>>>> following values: > > > >>>>>>>>>> + * 0: RGB444 > > > >>>>>>>>>> + * 1: RGB565 > > > >>>>>>>>>> + * 2: RGB666 > > > >>>>>>>>>> + * 3: RGB888 > > > >>>>>>>>> > > > >>>>>>>>> These are properties of the panel and should be obtained from > > > >>>>>>>>> the panel directly rather than an additional cell in this > > > >>>>>>>>> specifier. > > > >>>>>>>> > > > >>>>>>>> Okay. > > > >>>>>>>> What's the preferred way of doing this ? > > > >>>>>>>> What about defining an rgb-mode property in the panel node. > > > >>>>>>> > > > >>>>>>> There's .bpc in struct drm_display_info, I suspect that it could > > > >>>>>>> be used for this. Alternatively, maybe we could extend the list > > > >>>>>>> of color formats that go into drm_display_info.color_formats? > > > >>>>>>> RGB444 is already covered. > > > >>>>> > > > >>>>> I forgot to ask about bpc meaning. If, as I think, it means "bits > > > >>>>> per color" then it cannot be used to encode RGB565 where green > > > >>>>> color is encoded on 6 bits and red and blue are encoded on 5 bits. > > > >>>> > > > >>>> Yes, I agree that bps is not a good fit for what you need here. > > > >>> > > > >>> Okay, then I think we can replace bpc and color_formats by a > > > >>> bus_formats table containing all supported formats, and use an enum > > > >>> (something similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h > > > >>> [1]) to list the available formats. > > > >>> > > > >>> As this implies quite a few changes in crtc core and some drm drivers > > > >>> (nouveau, i915 and radeon), I'd like to be sure this is what both of > > > >>> you had in mind. > > > >> > > > >> I think it is, but just to make sure I understand you correctly, could > > > >> you just show how the drm_display_info structure would look like ? > > > > > > > > The new drm_display_info structure should look like this [2] (except > > > > that color_formats and bpc have not be removed yet), and [1] is just > > > > here to show how the video_bus_format enum would look like. > > > > > > > > [1] http://code.bulix.org/rfd0yx-86557 > > > > [2] http://code.bulix.org/7n03b4-86556 > > > > > > Quoting from your paste: > > > > > > + const enum video_bus_format *bus_formats; > > > + int nbus_formats; > > > > > > Do we really need more than one? > > > > We do if we want to replace the color_formats and bpc fields. > > > > Yes, that's what I was about to answer :-). Maybe we don't need to replace color_formats and bpc field immediately. That could be done in a follow-up patch. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/a5e6d501/attachment.sig>
[RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP driver
On Mon, Jul 21, 2014 at 04:58:25PM +0530, Ajay kumar wrote:
> On Mon, Jul 21, 2014 at 12:40 PM, Thierry Reding
> wrote:
> > On Fri, Jul 18, 2014 at 02:13:58AM +0530, Ajay Kumar wrote:
> >> From: Rahul Sharma
> >>
> >> This patch adds ps8622 lvds bridge discovery code to the dp driver.
> >>
> >> Signed-off-by: Rahul Sharma
> >> Signed-off-by: Ajay Kumar
> >> ---
> >> drivers/gpu/drm/exynos/exynos_dp_core.c |5 +
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> b/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> index 0ca6256..82e2942 100644
> >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> >> @@ -31,6 +31,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> #include "exynos_drm_drv.h"
> >> #include "exynos_dp_core.h"
> >> @@ -999,6 +1000,10 @@ static int exynos_drm_attach_lcd_bridge(struct
> >> exynos_dp_device *dp,
> >> if (find_bridge("nxp,ptn3460", &bridge)) {
> >> bridge_chain = ptn3460_init(dp->drm_dev, encoder,
> >> bridge.client,
> >> bridge.node);
> >> + } else if (find_bridge("parade,ps8622", &bridge) ||
> >> + find_bridge("parade,ps8625", &bridge)) {
> >> + bridge_chain = ps8622_init(dp->drm_dev, encoder,
> >> bridge.client,
> >> + bridge.node);
> >> }
> >
> > We really ought to be adding some sort of registry at some point.
> > Otherwise every driver that wants to use bridges needs to come up with a
> > similar set of helpers to instantiate them.
> >
> > Also you're making this driver depend on (now) two bridges, whereas it
> > really shouldn't matter which exact types it supports. Bridges should be
> > exposed via a generic interface.
>
> How about moving out the find_bridge() function into a common header file,
> and also supporting the list of bridge_init declarations in the same file?
>
> We get bridge chip node from phandle, and then pass the same node
> to find_bridge() which searches the list using of_device_is_compatible()
> to call the appropriate bridge_init function?
That could work, but it's still somewhat unusual and shouldn't be
required. I think we'd be better of with some sort of registry like we
have for panels. That would mean that a driver that wants to use a
bridge would do something like this:
struct drm_bridge *bridge;
struct device_node *np;
np = of_parse_phandle(dev->of_node, "bridge", 0);
if (np) {
bridge = of_drm_find_bridge(np);
of_node_put(np);
if (!bridge)
return -EPROBE_DEFER;
}
An alternative way would be to add a non-OF wrapper around this, like
this for example:
bridge = drm_bridge_get(dev, NULL);
Which would be conceptually the same as clk_get() or regulator_get() and
could be easily extended to support non-DT setups as well.
As for bridge drivers I think we may have to rework things a little, so
that a driver can call some sequence like this:
struct foo_bridge {
struct drm_bridge base;
...
};
static const struct drm_bridge_funcs foo_bridge_funcs = {
...
};
static int foo_probe(...)
{
struct foo_bridge *bridge;
int err;
bridge = devm_kzalloc(dev, sizeof(*bridge), GFP_KERNEL);
if (!bridge)
return -ENOMEM;
/* setup bridge (return -EPROBE_DEFER if necessary, ...) */
/* register bridge with DRM */
drm_bridge_init(&bridge->base);
bridge->base.dev = dev;
bridge->base.funcs = &foo_bridge_funcs;
err = drm_bridge_add(&bridge->base);
if (err < 0)
return err;
dev_set_drvdata(dev, bridge);
...
}
drm_bridge_add() would add the bridge to a global list of bridge devices
which drm_bridge_get()/of_drm_find_bridge() can use to find the one that
it needs. The above has the big advantage that it's completely
independent of the underlying bus that the bridge is on. It could be I2C
or SPI, platform device or PCI device.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/4dd28f51/attachment-0001.sig>
[Bug 81476] three monitors on two radeon cards works with some layouts not others
https://bugs.freedesktop.org/show_bug.cgi?id=81476 --- Comment #16 from Alex Deucher --- (In reply to comment #14) > > This looks like too much work; I'm not a graphics card programmer. It would > be faster for me to give away these two old FireMV cards and buy a single > modern card that Linux X11 supports with three 1600x1200 DVI monitors and > full 3D (no Xinerama). Alex - does AMD have anything like this? All evergreen (hd 5000 series) and newer dGPUs can support up to 6 monitors depending on the board. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/31bb6697/attachment.html>
[RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining
On Mon, Jul 21, 2014 at 05:28:13PM +0530, Ajay kumar wrote: > Hi Thierry, > > On Mon, Jul 21, 2014 at 1:52 PM, Thierry Reding > wrote: > > On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote: > > [...] > >> Also, remove the drm_connector implementation from ptn3460, > >> since the same is implemented using panel_binder. > > > > I think that's a step backwards. In fact I think the panel-bridge binder > > driver shouldn't be needed at all. At least not for now. We have a very > > limited number of bridge drivers, so it shouldn't hurt at this stage to > > implement the connector instantiation within each driver. Once we have > > more bridge drivers, and therefore a better understanding of what they > > need, we can always add something like the panel-binder (though I think > > it should be library code, similar to the drm_encoder_helper_add() API, > > rather than this kind of self-contained object). > panel_binder was needed to wrap around panel as a bridge, and this was in turn > needed because people wanted to represent a bridge+panel combo as a chain > of bridges. > So, if we choose to drop panel_binder, we choose to drop the idea of chaining > the bridges, and end up calling drm_panel functions directly from > individual bridges. > And, the bridge will implement the connector functions as you suggested. > I am okay with this, if Daniel/Rob don't have an issue with this. I think bridge chaining and panel handling are separate issues. That's why I think we shouldn't mix them like this. From earlier discussion[0] the conclusion was that the final element in the chain should implement a connector (with the appropriate type). Often that last element would be an encoder (when there are no bridges). Sometimes the last element would be a bridge. It's logical for that same element to also implement the panel integration since it's closely tied to the connector. Thierry [0]: http://lists.freedesktop.org/archives/dri-devel/2014-May/059685.html -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/6a685afb/attachment.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 14:16:42 +0200 Laurent Pinchart wrote: > Hi Thierry, > > On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > > On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 11:32:55 +0200 Laurent Pinchart wrote: > > >> On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > > >>> On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > > On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > > [...] > > > > >> + - atmel,panel: Should contain a phandle with 2 parameters. > > >> + The first cell is a phandle to a DRM panel device > > >> + The second cell encodes the RGB mode, which can take the > > >> following values: > > >> + * 0: RGB444 > > >> + * 1: RGB565 > > >> + * 2: RGB666 > > >> + * 3: RGB888 > > > > > > These are properties of the panel and should be obtained from > > > the panel directly rather than an additional cell in this > > > specifier. > > > > Okay. > > What's the preferred way of doing this ? > > What about defining an rgb-mode property in the panel node. > > >>> > > >>> There's .bpc in struct drm_display_info, I suspect that it could > > >>> be used for this. Alternatively, maybe we could extend the list > > >>> of color formats that go into drm_display_info.color_formats? > > >>> RGB444 is already covered. > > > > > > I forgot to ask about bpc meaning. If, as I think, it means "bits > > > per color" then it cannot be used to encode RGB565 where green > > > color is encoded on 6 bits and red and blue are encoded on 5 bits. > > > > Yes, I agree that bps is not a good fit for what you need here. > > >>> > > >>> Okay, then I think we can replace bpc and color_formats by a > > >>> bus_formats table containing all supported formats, and use an enum > > >>> (something similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h > > >>> [1]) to list the available formats. > > >>> > > >>> As this implies quite a few changes in crtc core and some drm drivers > > >>> (nouveau, i915 and radeon), I'd like to be sure this is what both of > > >>> you had in mind. > > >> > > >> I think it is, but just to make sure I understand you correctly, could > > >> you just show how the drm_display_info structure would look like ? > > > > > > The new drm_display_info structure should look like this [2] (except > > > that color_formats and bpc have not be removed yet), and [1] is just > > > here to show how the video_bus_format enum would look like. > > > > > > [1] http://code.bulix.org/rfd0yx-86557 > > > [2] http://code.bulix.org/7n03b4-86556 > > > > Quoting from your paste: > > > > + const enum video_bus_format *bus_formats; > > + int nbus_formats; > > > > Do we really need more than one? > > We do if we want to replace the color_formats and bpc fields. > Yes, that's what I was about to answer :-). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 14:15:16 +0200 Thierry Reding wrote: > On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote: > > Hi Thierry, > > > > Oops, I missed this reply. > > > > On Tue, 15 Jul 2014 12:31:37 +0200 > > Thierry Reding wrote: > > > > > On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote: > > > > On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding > > > gmail.com> wrote: > > > > > On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote: > > > [...] > > > > > > diff --git > > > > > > a/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > > > b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > > [...] > > > > > > +The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD > > > > > > device. > > > > > > +See Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt for more > > > > > > details. > > > > > > > > > > I think it's better to refer to these using relative filenames. When > > > > > the > > > > > device tree bindings are moved out of the kernel tree, they may no > > > > > longer use the same hierarchy. > > > > > > > > Sure. > > > > By relative path you mean ../../mfd/atmel-hlcdc.txt or just > > > > mfd/atmel-hlcdc.txt ? > > > > > > I think the former is more explicit. > > > > Okay. > > > > > > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters. > > > > > > + The first cell is a phandle to a DRM panel device > > > > > > + The second cell encodes the RGB mode, which can take the > > > > > > following values: > > > > > > + * 0: RGB444 > > > > > > + * 1: RGB565 > > > > > > + * 2: RGB666 > > > > > > + * 3: RGB888 > > > > > > > > > > These are properties of the panel and should be obtained from the > > > > > panel > > > > > directly rather than an additional cell in this specifier. > > > > > > > > Okay. > > > > What's the preferred way of doing this ? > > > > What about defining an rgb-mode property in the panel node. > > > > > > There's .bpc in struct drm_display_info, I suspect that it could be used > > > for this. Alternatively, maybe we could extend the list of color formats > > > that go into drm_display_info.color_formats? RGB444 is already covered. > > > > I don't think this color_formats field is intended to represent data > > stream format going through the bus. > > Moreover, AFAIU, RGB444 in this definition represent RGB 4:4:4 (chroma > > subsampling rate) and not 12 bits signals (4 bits for each color). > > > > Anyway I'll propose a patch series adding a new field to > > drm_display_info to encode the mediabus format (as discussed with > > Laurent and you). > > > > > > > > Also, like Laurent said, this shouldn't go into the device tree, since > > > it's already implied by the panel's compatible value, so we'd be > > > duplicating information. > > > > Again, this is not necessarily true (depending on your board design). > > One can decide to connect an RGB888 panel on an RGB666 bus and connect > > the missing pins to ground. > > I think in that case the board design itself could be considered as an > RGB888 to RGB666 bridge, and I think that's what the device tree should > be describing rather than a panel with a variable number of input > formats. So, you're suggesting to add an RGB to RGB drm_bridge driver (and the appropriate DT bindings) to handle this case, right ? I don't know much about drm bridges, but I'll take a look. Anyway, at the moment I don't have such hardware (one connecting an RGB888 panel on an RGB666 bus), I was just wondering how I could represent it ;-). Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Thierry, On Monday 21 July 2014 14:12:47 Thierry Reding wrote: > On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 11:32:55 +0200 Laurent Pinchart wrote: > >> On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > >>> On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > >>>> On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > >>>> [...] > >>>> > >>>>>>>>>> + - atmel,panel: Should contain a phandle with 2 parameters. > >>>>>>>>>> + The first cell is a phandle to a DRM panel device > >>>>>>>>>> + The second cell encodes the RGB mode, which can take the > >>>>>>>>>> following values: > >>>>>>>>>> + * 0: RGB444 > >>>>>>>>>> + * 1: RGB565 > >>>>>>>>>> + * 2: RGB666 > >>>>>>>>>> + * 3: RGB888 > >>>>>>>>> > >>>>>>>>> These are properties of the panel and should be obtained from > >>>>>>>>> the panel directly rather than an additional cell in this > >>>>>>>>> specifier. > >>>>>>>> > >>>>>>>> Okay. > >>>>>>>> What's the preferred way of doing this ? > >>>>>>>> What about defining an rgb-mode property in the panel node. > >>>>>>> > >>>>>>> There's .bpc in struct drm_display_info, I suspect that it could > >>>>>>> be used for this. Alternatively, maybe we could extend the list > >>>>>>> of color formats that go into drm_display_info.color_formats? > >>>>>>> RGB444 is already covered. > >>>>> > >>>>> I forgot to ask about bpc meaning. If, as I think, it means "bits > >>>>> per color" then it cannot be used to encode RGB565 where green > >>>>> color is encoded on 6 bits and red and blue are encoded on 5 bits. > >>>> > >>>> Yes, I agree that bps is not a good fit for what you need here. > >>> > >>> Okay, then I think we can replace bpc and color_formats by a > >>> bus_formats table containing all supported formats, and use an enum > >>> (something similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h > >>> [1]) to list the available formats. > >>> > >>> As this implies quite a few changes in crtc core and some drm drivers > >>> (nouveau, i915 and radeon), I'd like to be sure this is what both of > >>> you had in mind. > >> > >> I think it is, but just to make sure I understand you correctly, could > >> you just show how the drm_display_info structure would look like ? > > > > The new drm_display_info structure should look like this [2] (except > > that color_formats and bpc have not be removed yet), and [1] is just > > here to show how the video_bus_format enum would look like. > > > > [1] http://code.bulix.org/rfd0yx-86557 > > [2] http://code.bulix.org/7n03b4-86556 > > Quoting from your paste: > > + const enum video_bus_format *bus_formats; > + int nbus_formats; > > Do we really need more than one? We do if we want to replace the color_formats and bpc fields. -- Regards, Laurent Pinchart -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: This is a digitally signed message part. URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/69e9ee35/attachment-0001.sig>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Fri, Jul 18, 2014 at 04:51:52PM +0200, Boris BREZILLON wrote: > Hi Thierry, > > Oops, I missed this reply. > > On Tue, 15 Jul 2014 12:31:37 +0200 > Thierry Reding wrote: > > > On Tue, Jul 15, 2014 at 12:06:19PM +0200, Boris BREZILLON wrote: > > > On Mon, 14 Jul 2014 12:05:43 +0200 Thierry Reding > > gmail.com> wrote: > > > > On Mon, Jul 07, 2014 at 06:42:59PM +0200, Boris BREZILLON wrote: > > [...] > > > > > diff --git a/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > > b/Documentation/devicetree/bindings/drm/atmel-hlcdc-dc.txt > > > > [...] > > > > > +The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD > > > > > device. > > > > > +See Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt for more > > > > > details. > > > > > > > > I think it's better to refer to these using relative filenames. When the > > > > device tree bindings are moved out of the kernel tree, they may no > > > > longer use the same hierarchy. > > > > > > Sure. > > > By relative path you mean ../../mfd/atmel-hlcdc.txt or just > > > mfd/atmel-hlcdc.txt ? > > > > I think the former is more explicit. > > Okay. > > > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters. > > > > > + The first cell is a phandle to a DRM panel device > > > > > + The second cell encodes the RGB mode, which can take the > > > > > following values: > > > > > + * 0: RGB444 > > > > > + * 1: RGB565 > > > > > + * 2: RGB666 > > > > > + * 3: RGB888 > > > > > > > > These are properties of the panel and should be obtained from the panel > > > > directly rather than an additional cell in this specifier. > > > > > > Okay. > > > What's the preferred way of doing this ? > > > What about defining an rgb-mode property in the panel node. > > > > There's .bpc in struct drm_display_info, I suspect that it could be used > > for this. Alternatively, maybe we could extend the list of color formats > > that go into drm_display_info.color_formats? RGB444 is already covered. > > I don't think this color_formats field is intended to represent data > stream format going through the bus. > Moreover, AFAIU, RGB444 in this definition represent RGB 4:4:4 (chroma > subsampling rate) and not 12 bits signals (4 bits for each color). > > Anyway I'll propose a patch series adding a new field to > drm_display_info to encode the mediabus format (as discussed with > Laurent and you). > > > > > Also, like Laurent said, this shouldn't go into the device tree, since > > it's already implied by the panel's compatible value, so we'd be > > duplicating information. > > Again, this is not necessarily true (depending on your board design). > One can decide to connect an RGB888 panel on an RGB666 bus and connect > the missing pins to ground. I think in that case the board design itself could be considered as an RGB888 to RGB666 bridge, and I think that's what the device tree should be describing rather than a panel with a variable number of input formats. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/c0a93c42/attachment.sig>
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 08:42:58PM +0300, Oded Gabbay wrote: > On 21/07/14 18:54, Jerome Glisse wrote: > > On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: > >> On 21/07/14 16:39, Christian K?nig wrote: > >>> Am 21.07.2014 14:36, schrieb Oded Gabbay: > On 20/07/14 20:46, Jerome Glisse wrote: > > On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > >> Forgot to cc mailing list on cover letter. Sorry. > >> > >> As a continuation to the existing discussion, here is a v2 patch series > >> restructured with a cleaner history and no > >> totally-different-early-versions > >> of the code. > >> > >> Instead of 83 patches, there are now a total of 25 patches, where 5 of > >> them > >> are modifications to radeon driver and 18 of them include only amdkfd > >> code. > >> There is no code going away or even modified between patches, only > >> added. > >> > >> The driver was renamed from radeon_kfd to amdkfd and moved to reside > >> under > >> drm/radeon/amdkfd. This move was done to emphasize the fact that this > >> driver > >> is an AMD-only driver at this point. Having said that, we do foresee a > >> generic hsa framework being implemented in the future and in that > >> case, we > >> will adjust amdkfd to work within that framework. > >> > >> As the amdkfd driver should support multiple AMD gfx drivers, we want > >> to > >> keep it as a seperate driver from radeon. Therefore, the amdkfd code is > >> contained in its own folder. The amdkfd folder was put under the radeon > >> folder because the only AMD gfx driver in the Linux kernel at this > >> point > >> is the radeon driver. Having said that, we will probably need to move > >> it > >> (maybe to be directly under drm) after we integrate with additional > >> AMD gfx > >> drivers. > >> > >> For people who like to review using git, the v2 patch set is located > >> at: > >> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > >> > >> Written by Oded Gabbayh > > > > So quick comments before i finish going over all patches. There is many > > things that need more documentation espacialy as of right now there is > > no userspace i can go look at. > So quick comments on some of your questions but first of all, thanks for > the > time you dedicated to review the code. > > > > There few show stopper, biggest one is gpu memory pinning this is a big > > no, that would need serious arguments for any hope of convincing me on > > that side. > We only do gpu memory pinning for kernel objects. There are no userspace > objects that are pinned on the gpu memory in our driver. If that is the > case, > is it still a show stopper ? > > The kernel objects are: > - pipelines (4 per device) > - mqd per hiq (only 1 per device) > - mqd per userspace queue. On KV, we support up to 1K queues per > process, for > a total of 512K queues. Each mqd is 151 bytes, but the allocation is > done in > 256 alignment. So total *possible* memory is 128MB > - kernel queue (only 1 per device) > - fence address for kernel queue > - runlists for the CP (1 or 2 per device) > >>> > >>> The main questions here are if it's avoid able to pin down the memory and > >>> if the > >>> memory is pinned down at driver load, by request from userspace or by > >>> anything > >>> else. > >>> > >>> As far as I can see only the "mqd per userspace queue" might be a bit > >>> questionable, everything else sounds reasonable. > >>> > >>> Christian. > >> > >> Most of the pin downs are done on device initialization. > >> The "mqd per userspace" is done per userspace queue creation. However, as I > >> said, it has an upper limit of 128MB on KV, and considering the 2G local > >> memory, I think it is OK. > >> The runlists are also done on userspace queue creation/deletion, but we > >> only > >> have 1 or 2 runlists per device, so it is not that bad. > > > > 2G local memory ? You can not assume anything on userside configuration some > > one might build an hsa computer with 512M and still expect a functioning > > desktop. > First of all, I'm only considering Kaveri computer, not "hsa" computer. > Second, I would imagine we can build some protection around it, like > checking total local memory and limit number of queues based on some > percentage of that total local memory. So, if someone will have only > 512M, he will be able to open less queues. > > > > > > I need to go look into what all this mqd is for, what it does and what it is > > about. But pinning is really bad and this is an issue with userspace command > > scheduling an issue that obviously AMD fails to take into account in design > > phase. > Maybe, but that is the H/W design non-the-less. We can't very well > change the
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, Jul 21, 2014 at 11:57:37AM +0200, Boris BREZILLON wrote: > On Mon, 21 Jul 2014 11:32:55 +0200 > Laurent Pinchart wrote: > > > Hi Boris, > > > > On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > > > On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > > > > On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > > > > [...] > > > > > > > >>>>>>> + - atmel,panel: Should contain a phandle with 2 parameters. > > > >>>>>>> + The first cell is a phandle to a DRM panel device > > > >>>>>>> + The second cell encodes the RGB mode, which can take the > > > >>>>>>> following values: > > > >>>>>>> + * 0: RGB444 > > > >>>>>>> + * 1: RGB565 > > > >>>>>>> + * 2: RGB666 > > > >>>>>>> + * 3: RGB888 > > > >>>>>> > > > >>>>>> These are properties of the panel and should be obtained from > > > >>>>>> the panel directly rather than an additional cell in this > > > >>>>>> specifier. > > > >>>>> > > > >>>>> Okay. > > > >>>>> What's the preferred way of doing this ? > > > >>>>> What about defining an rgb-mode property in the panel node. > > > >>>> > > > >>>> There's .bpc in struct drm_display_info, I suspect that it could be > > > >>>> used for this. Alternatively, maybe we could extend the list of color > > > >>>> formats that go into drm_display_info.color_formats? RGB444 is > > > >>>> already > > > >>>> covered. > > > >> > > > >> I forgot to ask about bpc meaning. If, as I think, it means "bits per > > > >> color" then it cannot be used to encode RGB565 where green color is > > > >> encoded on 6 bits and red and blue are encoded on 5 bits. > > > > > > > > Yes, I agree that bps is not a good fit for what you need here. > > > > > > Okay, then I think we can replace bpc and color_formats by a bus_formats > > > table containing all supported formats, and use an enum (something > > > similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h [1]) to list > > > the available formats. > > > > > > As this implies quite a few changes in crtc core and some drm drivers > > > (nouveau, i915 and radeon), I'd like to be sure this is what both of you > > > had in mind. > > > > I think it is, but just to make sure I understand you correctly, could you > > just show how the drm_display_info structure would look like ? > > > > The new drm_display_info structure should look like this [2] (except > that color_formats and bpc have not be removed yet), and [1] is just > here to show how the video_bus_format enum would look like. > > [1] http://code.bulix.org/rfd0yx-86557 > [2] http://code.bulix.org/7n03b4-86556 Quoting from your paste: + const enum video_bus_format *bus_formats; + int nbus_formats; Do we really need more than one? Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/309fb551/attachment.sig>
[PATCH] drm/nouveau/disp: Use NULL for pointers
From: Thierry Reding The return type of exec_lookup() is struct nvkm_output *, so it should return NULL rather than 0. Signed-off-by: Thierry Reding --- drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c index fa30d8196f35..ebf64e1d0a70 100644 --- a/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c +++ b/drivers/gpu/drm/nouveau/core/engine/disp/nvd0.c @@ -939,7 +939,7 @@ exec_lookup(struct nv50_disp_priv *priv, int head, int or, u32 ctrl, case 0x0900: type = DCB_OUTPUT_DP; mask = 2; break; default: nv_error(priv, "unknown SOR mc 0x%08x\n", ctrl); - return 0x; + return NULL; } } -- 2.0.1
[PATCH] drm: Add missing __user annotation
Hi
On Mon, Jul 21, 2014 at 1:24 PM, Thierry Reding
wrote:
> From: Thierry Reding
>
> The drm_copy_field() function copies strings into userspace buffers, so
> the first parameter needs to have a __user annotation to avoid warnings
> from the sparse checker.
Reviewed-by: David Herrmann
Thanks
David
> Signed-off-by: Thierry Reding
> ---
> drivers/gpu/drm/drm_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8218078b6133..0cc182745e31 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -233,7 +233,7 @@ module_exit(drm_core_exit);
> /**
> * Copy and IOCTL return string to user space
> */
> -static int drm_copy_field(char *buf, size_t *buf_len, const char *value)
> +static int drm_copy_field(char __user *buf, size_t *buf_len, const char
> *value)
> {
> int len;
>
> --
> 2.0.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 81579] VDPAU: videos slow to a crawl when more than 1 video is playing
https://bugs.freedesktop.org/show_bug.cgi?id=81579 Christian K?nig changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTABUG --- Comment #7 from Christian K?nig --- >[1.093019] == power state 2 == >[1.093019] ui class: none >[1.093020] internal class: uvd >[1.093021] caps: video >[1.093022] uvdvclk: 48000 dclk: 38000 >[1.093023] power level 0sclk: 5 mclk: 8 vddc: >1000 >[1.093024] power level 1sclk: 5 mclk: 8 vddc: >1000 >[1.093025] power level 2sclk: 5 mclk: 8 vddc: >1000 The power table indeed has only a single UVD performance level and that doesn't looks like it is suitable for handling two 1080p streams at the same time. You need at least a vclk of 533Mhz and a dclk of 400Mhz for that. You can try to hack into the kernel and overclock the UVD block, but I don't advise to do so. OEMs usually have a good reason for limiting the clocks to a lower level. Anyway that's clearly not a driver bug, cause the kernel does what it is supposed to do. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/a08ca840/attachment.html>
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 Christian K?nig changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #7 from Christian K?nig --- Ah, stop my fault. That was for the other bug report. Sorry, I've just messed those two up. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/4cb017b2/attachment.html>
[PATCH] drm/radeon: remove discardable flag from radeon_gem_object_create
From: Christian K?nig
Unused and unimplemented. Also fix specifying the
kernel flag incorrectly at one occasion.
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon.h | 2 +-
drivers/gpu/drm/radeon/radeon_fb.c | 3 +--
drivers/gpu/drm/radeon/radeon_gem.c | 7 +++
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 6ce80ae..9b6fbda 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -548,7 +548,7 @@ int radeon_gem_init(struct radeon_device *rdev);
void radeon_gem_fini(struct radeon_device *rdev);
int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
int alignment, int initial_domain,
- u32 flags, bool discardable, bool kernel,
+ u32 flags, bool kernel,
struct drm_gem_object **obj);
int radeon_mode_dumb_create(struct drm_file *file_priv,
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c
b/drivers/gpu/drm/radeon/radeon_fb.c
index 477ea0d..94b0f2a 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -127,8 +127,7 @@ static int radeonfb_create_pinned_object(struct
radeon_fbdev *rfbdev,
aligned_size = ALIGN(size, PAGE_SIZE);
ret = radeon_gem_object_create(rdev, aligned_size, 0,
RADEON_GEM_DOMAIN_VRAM,
- 0, false, true,
- &gobj);
+ 0, true, &gobj);
if (ret) {
printk(KERN_ERR "failed to allocate framebuffer (%d)\n",
aligned_size);
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
b/drivers/gpu/drm/radeon/radeon_gem.c
index eafe316..9fb1c06 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -42,7 +42,7 @@ void radeon_gem_object_free(struct drm_gem_object *gobj)
int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
int alignment, int initial_domain,
- u32 flags, bool discardable, bool kernel,
+ u32 flags, bool kernel,
struct drm_gem_object **obj)
{
struct radeon_bo *robj;
@@ -255,7 +255,7 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void
*data,
args->size = roundup(args->size, PAGE_SIZE);
r = radeon_gem_object_create(rdev, args->size, args->alignment,
args->initial_domain, args->flags,
-false, false, &gobj);
+false, &gobj);
if (r) {
up_read(&rdev->exclusive_lock);
r = radeon_gem_handle_lockup(rdev, r);
@@ -570,8 +570,7 @@ int radeon_mode_dumb_create(struct drm_file *file_priv,
r = radeon_gem_object_create(rdev, args->size, 0,
RADEON_GEM_DOMAIN_VRAM, 0,
-false, ttm_bo_type_device,
-&gobj);
+false, &gobj);
if (r)
return -ENOMEM;
--
1.9.1
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 --- Comment #6 from Christian K?nig --- (In reply to comment #5) > Is it fixed in 3.16-rc6? That fix should be in all kernels since 3.14. It was named "drm/radeon: fix UVD IRQ support on SI". -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/06f7d3dc/attachment.html>
[PATCH] drm: Add missing __user annotation
From: Thierry Reding
The drm_copy_field() function copies strings into userspace buffers, so
the first parameter needs to have a __user annotation to avoid warnings
from the sparse checker.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8218078b6133..0cc182745e31 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -233,7 +233,7 @@ module_exit(drm_core_exit);
/**
* Copy and IOCTL return string to user space
*/
-static int drm_copy_field(char *buf, size_t *buf_len, const char *value)
+static int drm_copy_field(char __user *buf, size_t *buf_len, const char *value)
{
int len;
--
2.0.1
[PATCH] drm/dp: Staticize a couple of DP utility functions
From: Thierry Reding
sparse complains about these functions missing a prototype, but looking
closer they aren't (and likely not supposed to be) used outside of this
source file, so they can be made static.
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/drm_dp_mst_topology.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 369d6c49145b..c9a772554308 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1482,10 +1482,10 @@ static int drm_dp_send_enum_path_resources(struct
drm_dp_mst_topology_mgr *mgr,
return 0;
}
-int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_mst_port *port,
- int id,
- int pbn)
+static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_port *port,
+ int id,
+ int pbn)
{
struct drm_dp_sideband_msg_tx *txmsg;
struct drm_dp_mst_branch *mstb;
@@ -1536,10 +1536,10 @@ static int drm_dp_create_payload_step1(struct
drm_dp_mst_topology_mgr *mgr,
return 0;
}
-int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_mst_port *port,
- int id,
- struct drm_dp_payload *payload)
+static int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_port *port,
+ int id,
+ struct drm_dp_payload *payload)
{
int ret;
ret = drm_dp_payload_send_msg(mgr, port, id, port->vcpi.pbn);
@@ -1549,10 +1549,10 @@ int drm_dp_create_payload_step2(struct
drm_dp_mst_topology_mgr *mgr,
return ret;
}
-int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
-struct drm_dp_mst_port *port,
-int id,
-struct drm_dp_payload *payload)
+static int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_mst_port *port,
+ int id,
+ struct drm_dp_payload *payload)
{
DRM_DEBUG_KMS("\n");
/* its okay for these to fail */
@@ -1565,9 +1565,9 @@ int drm_dp_destroy_payload_step1(struct
drm_dp_mst_topology_mgr *mgr,
return 0;
}
-int drm_dp_destroy_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
-int id,
-struct drm_dp_payload *payload)
+static int drm_dp_destroy_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
+ int id,
+ struct drm_dp_payload *payload)
{
payload->payload_state = 0;
return 0;
--
2.0.1
[PATCH v6 10/14] drm/panel: add S6E3FA0 driver
On 07/21/2014 11:19 AM, Thierry Reding wrote:
> On Mon, Jul 21, 2014 at 10:56:08AM +0200, Andrzej Hajda wrote:
>> On 07/18/2014 03:49 AM, YoungJun Cho wrote:
>>> Hi Thierry,
>>>
>>> Thank you a lot for kind comments.
>>>
>>> On 07/17/2014 07:36 PM, Thierry Reding wrote:
On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
[...]
> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
> b/drivers/gpu/drm/panel/panel-s6e3fa0.c
[...]
> +/* Manufacturer Command Set */
> +#define MCS_GLOBAL_PARAMETER 0xb0
> +#define MCS_AID 0xb2
> +#define MCS_ELVSSOPT 0xb6
> +#define MCS_TEMPERATURE_SET 0xb8
> +#define MCS_PENTILE_CTRL 0xc0
> +#define MCS_GAMMA_MODE 0xca
> +#define MCS_VDDM 0xd7
> +#define MCS_ALS 0xe3
> +#define MCS_ERR_FG 0xed
> +#define MCS_KEY_LEV1 0xf0
> +#define MCS_GAMMA_UPDATE 0xf7
> +#define MCS_KEY_LEV2 0xfc
> +#define MCS_RE 0xfe
> +#define MCS_TOUT2_HSYNC 0xff
> +
> +/* Content Adaptive Brightness Control */
> +#define DCS_WRITE_CABC 0x55
Is this not a manufacturer specific command? I couldn't find it in the
DSI or DCS specifications, but it sounds like something standard (also
indicated by the DCS_ prefix). Can you point out the specification for
this?
>>> Andrzej commented before and decided it as DCS one because if the value
>>> is less than 0xb0, it is DCS one and the others are MCS one.
>>> But still I'm not sure it is correct.
>> I would not say 'decided'. I have just stated that according to DCS
>> specification
>> it should be DCS command (below 0xb0), but it is not present in mipi dcs
>> specs.
>> On the other side many panels have it [1]:
>>
>> [1]:
>> https://www.google.com/search?q=%22Write+Content+Adaptive+Brightness+Control%22
>
> Yeah, my search yielded similar results. I wonder if this is perhaps
> part of a draft future specification. I'll try to ask around some more
> if anybody knows what the status of this is.
>
> +static void s6e3fa0_read_mtp_id(struct s6e3fa0 *ctx)
> +{
> + unsigned char id[MTP_ID_LEN];
> + int ret;
> +
> + s6e3fa0_set_maximum_return_packet_size(ctx, MTP_ID_LEN);
> + ret = s6e3fa0_dcs_read(ctx, MIPI_DCS_GET_DISPLAY_ID, id, MTP_ID_LEN);
This also looks like a standard DCS command. I can't find it in either
v1.01 nor v1.02 of the specification, though. Do you know where it's
specified?
>>> Yes, I also can't find it in DCS specification and there is no special
>>> description in panel datasheet.
>>> But as it is declared in "include/video/mipi_display.h", so I think it
>>> is a standard one.
>>
>> On page 9 of the "Introduction of MIPI by Renesas" [2] there is info
>> it is a part of "Nokia I/F command list", I guess it is kind of alpha
>> version of MIPI DCS.
>>
>> [2]: http://wenku.baidu.com/view/658fab68af1ffc4ffe47acbe.html
>
> That link is the only one which contains "Nokia I/F command list" on the
> internet (that is, according to Google). But since this is already part
> of the mipi_display.h header file we may as well use it.
>
> I wonder if perhaps the READ_DDB_START and READ_DDB_CONTINUE commands
> could be used as a replacement for this display ID.
>
> Adding Guennadi, Tomi, Paul and Imre on Cc since they were involved with
> the original patch that added mipi_display.h. Perhaps they remember what
> the origin of this command is.
I guess it was PCF8833 used in Nokia 6100 [1][2].
[1]: http://www.vintagecomputercables.com/datasheet/PCF8833_1.pdf
[2]:
http://www.elecfreaks.com/store/download/datasheet/shield/6100_Display_Driver.pdf
Regards
Andrzej
>
> Thierry
>
[PATCH] drm/ttm: Fix a few sparse warnings
From: Thierry Reding
The final parameter to ttm_bo_reserve() is a pointer, therefore callers
should use NULL instead of 0.
Fixes a bunch of sparse warnings of this type:
warning: Using plain integer as NULL pointer
Signed-off-by: Thierry Reding
---
drivers/gpu/drm/ast/ast_drv.h| 2 +-
drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
drivers/gpu/drm/bochs/bochs_kms.c| 4 ++--
drivers/gpu/drm/cirrus/cirrus_drv.h | 2 +-
drivers/gpu/drm/mgag200/mgag200_drv.h| 2 +-
drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +++---
drivers/gpu/drm/nouveau/nouveau_gem.c| 4 ++--
drivers/gpu/drm/qxl/qxl_object.h | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 6 +++---
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 2 +-
12 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 5d6a87573c33..957d4fabf1e1 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -362,7 +362,7 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool
no_wait)
{
int ret;
- ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
if (ret) {
if (ret != -ERESTARTSYS && ret != -EBUSY)
DRM_ERROR("reserve failed %p\n", bo);
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
b/drivers/gpu/drm/bochs/bochs_fbdev.c
index 19cf3e9413b6..fe95d31cd110 100644
--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -72,7 +72,7 @@ static int bochsfb_create(struct drm_fb_helper *helper,
bo = gem_to_bochs_bo(gobj);
- ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
b/drivers/gpu/drm/bochs/bochs_kms.c
index dcf2e55f4ae9..9abd2312f5bd 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -53,7 +53,7 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc,
int x, int y,
if (old_fb) {
bochs_fb = to_bochs_framebuffer(old_fb);
bo = gem_to_bochs_bo(bochs_fb->obj);
- ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
if (ret) {
DRM_ERROR("failed to reserve old_fb bo\n");
} else {
@@ -67,7 +67,7 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc,
int x, int y,
bochs_fb = to_bochs_framebuffer(crtc->primary->fb);
bo = gem_to_bochs_bo(bochs_fb->obj);
- ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 117d3eca5e37..401c890b6c6a 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -241,7 +241,7 @@ static inline int cirrus_bo_reserve(struct cirrus_bo *bo,
bool no_wait)
{
int ret;
- ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
if (ret) {
if (ret != -ERESTARTSYS && ret != -EBUSY)
DRM_ERROR("reserve failed %p\n", bo);
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index cf11ee68a6d9..80de23d9b9c9 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -280,7 +280,7 @@ static inline int mgag200_bo_reserve(struct mgag200_bo *bo,
bool no_wait)
{
int ret;
- ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
+ ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
if (ret) {
if (ret != -ERESTARTSYS && ret != -EBUSY)
DRM_ERROR("reserve failed %p\n", bo);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index f00ae18003f1..58e49dcf5bd8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -310,7 +310,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t memtype)
struct ttm_buffer_object *bo = &nvbo->bo;
int ret;
- ret = ttm_bo_reserve(bo, false, false, false, 0);
+ ret = ttm_bo_reserve(bo, false, false, false, NULL);
if (ret)
goto out;
@@ -351,7 +351,7 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
struct ttm_buffer_object *bo = &nvbo->bo;
int ret, ref;
- ret = ttm_bo_reserve(bo, false, false, false, 0);
+ ret = ttm_bo_reserve(bo, f
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 --- Comment #5 from darkbasic --- Is it fixed in 3.16-rc6? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/601c6d29/attachment.html>
[PATCH 0/3] drm/exynos: Allow module to be autoloaded
On 2014? 07? 19? 05:36, Sjoerd Simons wrote: > The exynos DRM module currently is not automatically loaded when build as a > module. This is due to the simple fact that it doesn't have any > MODULE_DEVICE_TABLE entries whatsoever... Most of these were removed > previously > as it wasn't possible at the time to have multiple calls to > MODULE_DEVICE_TABLE > in one module, however commit 21bdd17b21b45ea solved that. > > The first two patches revert the previous removals of MODULE_DEVICE_TABLE > calls, while the last one adds calls for the remaining OF match tables > without a > MODULE_DEVICE_TABLE call. Hi, Exynos drm follows single-driver model. So each usb driver of Exynos drm wouldn't need its own MODULE_DEVICE_TABLE. Thanks, Inki Dae > > Sjoerd Simons (3): > Revert "drm/exynos: fix module build error" > Revert "drm/exynos: remove MODULE_DEVICE_TABLE definitions" > drm/exynos: Add MODULE_DEVICE_TABLE entries for various components > > drivers/gpu/drm/exynos/exynos_dp_core.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_fimc.c| 1 + > drivers/gpu/drm/exynos/exynos_drm_fimd.c| 1 + > drivers/gpu/drm/exynos/exynos_drm_g2d.c | 1 + > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 1 + > drivers/gpu/drm/exynos/exynos_hdmi.c| 1 + > drivers/gpu/drm/exynos/exynos_mixer.c | 1 + > 8 files changed, 8 insertions(+) >
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 05:25:11PM +0200, Daniel Vetter wrote: > On Mon, Jul 21, 2014 at 03:39:09PM +0200, Christian K?nig wrote: > > Am 21.07.2014 14:36, schrieb Oded Gabbay: > > >On 20/07/14 20:46, Jerome Glisse wrote: > > >>On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > > >>>Forgot to cc mailing list on cover letter. Sorry. > > >>> > > >>>As a continuation to the existing discussion, here is a v2 patch series > > >>>restructured with a cleaner history and no > > >>>totally-different-early-versions > > >>>of the code. > > >>> > > >>>Instead of 83 patches, there are now a total of 25 patches, where 5 of > > >>>them > > >>>are modifications to radeon driver and 18 of them include only amdkfd > > >>>code. > > >>>There is no code going away or even modified between patches, only > > >>>added. > > >>> > > >>>The driver was renamed from radeon_kfd to amdkfd and moved to reside > > >>>under > > >>>drm/radeon/amdkfd. This move was done to emphasize the fact that this > > >>>driver > > >>>is an AMD-only driver at this point. Having said that, we do foresee a > > >>>generic hsa framework being implemented in the future and in that > > >>>case, we > > >>>will adjust amdkfd to work within that framework. > > >>> > > >>>As the amdkfd driver should support multiple AMD gfx drivers, we want > > >>>to > > >>>keep it as a seperate driver from radeon. Therefore, the amdkfd code is > > >>>contained in its own folder. The amdkfd folder was put under the radeon > > >>>folder because the only AMD gfx driver in the Linux kernel at this > > >>>point > > >>>is the radeon driver. Having said that, we will probably need to move > > >>>it > > >>>(maybe to be directly under drm) after we integrate with additional > > >>>AMD gfx > > >>>drivers. > > >>> > > >>>For people who like to review using git, the v2 patch set is located > > >>>at: > > >>>http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > > >>> > > >>>Written by Oded Gabbayh > > >> > > >>So quick comments before i finish going over all patches. There is many > > >>things that need more documentation espacialy as of right now there is > > >>no userspace i can go look at. > > >So quick comments on some of your questions but first of all, thanks for > > >the time you dedicated to review the code. > > >> > > >>There few show stopper, biggest one is gpu memory pinning this is a big > > >>no, that would need serious arguments for any hope of convincing me on > > >>that side. > > >We only do gpu memory pinning for kernel objects. There are no userspace > > >objects that are pinned on the gpu memory in our driver. If that is the > > >case, is it still a show stopper ? > > > > > >The kernel objects are: > > >- pipelines (4 per device) > > >- mqd per hiq (only 1 per device) > > >- mqd per userspace queue. On KV, we support up to 1K queues per process, > > >for a total of 512K queues. Each mqd is 151 bytes, but the allocation is > > >done in 256 alignment. So total *possible* memory is 128MB > > >- kernel queue (only 1 per device) > > >- fence address for kernel queue > > >- runlists for the CP (1 or 2 per device) > > > > The main questions here are if it's avoid able to pin down the memory and if > > the memory is pinned down at driver load, by request from userspace or by > > anything else. > > > > As far as I can see only the "mqd per userspace queue" might be a bit > > questionable, everything else sounds reasonable. > > Aside, i915 perspective again (i.e. how we solved this): When scheduling > away from contexts we unpin them and put them into the lru. And in the > shrinker we have a last-ditch callback to switch to a default context > (since you can't ever have no context once you've started) which means we > can evict any context object if it's getting in the way. So Intel hardware report through some interrupt or some channel when it is not using a context ? ie kernel side get notification when some user context is done executing ? The issue with radeon hardware AFAICT is that the hardware do not report any thing about the userspace context running ie you do not get notification when a context is not use. Well AFAICT. Maybe hardware do provide that. Like the VMID is a limited resources so you have to dynamicly bind them so maybe we can only allocate pinned buffer for each VMID and then when binding a PASID to a VMID it also copy back pinned buffer to pasid unpinned copy. Cheers, J?r?me > > We must do that since the contexts have to be in global gtt, which is > shared for scanouts. So fragmenting that badly with lots of context > objects and other stuff is a no-go, since that means we'll start to fail > pageflips. > > I don't know whether ttm has a ready-made concept for such > opportunistically pinned stuff. I guess you could wire up the "switch to > dflt context" action to the evict/move function if ttm wants to get rid of > the currently used hw context. > > Oh and: This is another reason for letting the kernel schedule contexts, > s
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 11:32:55 +0200 Laurent Pinchart wrote: > Hi Boris, > > On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > > On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > > > On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > > > [...] > > > > > >>> + - atmel,panel: Should contain a phandle with 2 parameters. > > >>> + The first cell is a phandle to a DRM panel device > > >>> + The second cell encodes the RGB mode, which can take the > > >>> following values: > > >>> + * 0: RGB444 > > >>> + * 1: RGB565 > > >>> + * 2: RGB666 > > >>> + * 3: RGB888 > > >> > > >> These are properties of the panel and should be obtained from > > >> the panel directly rather than an additional cell in this specifier. > > > > > > Okay. > > > What's the preferred way of doing this ? > > > What about defining an rgb-mode property in the panel node. > > > > There's .bpc in struct drm_display_info, I suspect that it could be > > used for this. Alternatively, maybe we could extend the list of color > > formats that go into drm_display_info.color_formats? RGB444 is already > > covered. > > >> > > >> I forgot to ask about bpc meaning. If, as I think, it means "bits per > > >> color" then it cannot be used to encode RGB565 where green color is > > >> encoded on 6 bits and red and blue are encoded on 5 bits. > > > > > > Yes, I agree that bps is not a good fit for what you need here. > > > > Okay, then I think we can replace bpc and color_formats by a bus_formats > > table containing all supported formats, and use an enum (something > > similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h [1]) to list > > the available formats. > > > > As this implies quite a few changes in crtc core and some drm drivers > > (nouveau, i915 and radeon), I'd like to be sure this is what both of you > > had in mind. > > I think it is, but just to make sure I understand you correctly, could you > just show how the drm_display_info structure would look like ? > The new drm_display_info structure should look like this [2] (except that color_formats and bpc have not be removed yet), and [1] is just here to show how the video_bus_format enum would look like. [1] http://code.bulix.org/rfd0yx-86557 [2] http://code.bulix.org/7n03b4-86556 -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH v2 00/25] AMDKFD kernel driver
On Mon, Jul 21, 2014 at 05:12:06PM +0300, Oded Gabbay wrote: > On 21/07/14 16:39, Christian K?nig wrote: > >Am 21.07.2014 14:36, schrieb Oded Gabbay: > >>On 20/07/14 20:46, Jerome Glisse wrote: > >>>On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: > Forgot to cc mailing list on cover letter. Sorry. > > As a continuation to the existing discussion, here is a v2 patch series > restructured with a cleaner history and no > totally-different-early-versions > of the code. > > Instead of 83 patches, there are now a total of 25 patches, where 5 of > them > are modifications to radeon driver and 18 of them include only amdkfd > code. > There is no code going away or even modified between patches, only added. > > The driver was renamed from radeon_kfd to amdkfd and moved to reside under > drm/radeon/amdkfd. This move was done to emphasize the fact that this > driver > is an AMD-only driver at this point. Having said that, we do foresee a > generic hsa framework being implemented in the future and in that case, we > will adjust amdkfd to work within that framework. > > As the amdkfd driver should support multiple AMD gfx drivers, we want to > keep it as a seperate driver from radeon. Therefore, the amdkfd code is > contained in its own folder. The amdkfd folder was put under the radeon > folder because the only AMD gfx driver in the Linux kernel at this point > is the radeon driver. Having said that, we will probably need to move it > (maybe to be directly under drm) after we integrate with additional AMD > gfx > drivers. > > For people who like to review using git, the v2 patch set is located at: > http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 > > Written by Oded Gabbayh > >>> > >>>So quick comments before i finish going over all patches. There is many > >>>things that need more documentation espacialy as of right now there is > >>>no userspace i can go look at. > >>So quick comments on some of your questions but first of all, thanks for the > >>time you dedicated to review the code. > >>> > >>>There few show stopper, biggest one is gpu memory pinning this is a big > >>>no, that would need serious arguments for any hope of convincing me on > >>>that side. > >>We only do gpu memory pinning for kernel objects. There are no userspace > >>objects that are pinned on the gpu memory in our driver. If that is the > >>case, > >>is it still a show stopper ? > >> > >>The kernel objects are: > >>- pipelines (4 per device) > >>- mqd per hiq (only 1 per device) > >>- mqd per userspace queue. On KV, we support up to 1K queues per process, > >>for > >>a total of 512K queues. Each mqd is 151 bytes, but the allocation is done in > >>256 alignment. So total *possible* memory is 128MB > >>- kernel queue (only 1 per device) > >>- fence address for kernel queue > >>- runlists for the CP (1 or 2 per device) > > > >The main questions here are if it's avoid able to pin down the memory and if > >the > >memory is pinned down at driver load, by request from userspace or by > >anything > >else. > > > >As far as I can see only the "mqd per userspace queue" might be a bit > >questionable, everything else sounds reasonable. > > > >Christian. > > Most of the pin downs are done on device initialization. > The "mqd per userspace" is done per userspace queue creation. However, as I > said, it has an upper limit of 128MB on KV, and considering the 2G local > memory, I think it is OK. > The runlists are also done on userspace queue creation/deletion, but we only > have 1 or 2 runlists per device, so it is not that bad. 2G local memory ? You can not assume anything on userside configuration some one might build an hsa computer with 512M and still expect a functioning desktop. I need to go look into what all this mqd is for, what it does and what it is about. But pinning is really bad and this is an issue with userspace command scheduling an issue that obviously AMD fails to take into account in design phase. > Oded > > > >>> > >>>It might be better to add a drivers/gpu/drm/amd directory and add common > >>>stuff there. > >>> > >>>Given that this is not intended to be final HSA api AFAICT then i would > >>>say this far better to avoid the whole kfd module and add ioctl to radeon. > >>>This would avoid crazy communication btw radeon and kfd. > >>> > >>>The whole aperture business needs some serious explanation. Especialy as > >>>you want to use userspace address there is nothing to prevent userspace > >>>program from allocating things at address you reserve for lds, scratch, > >>>... only sane way would be to move those lds, scratch inside the virtual > >>>address reserved for kernel (see kernel memory map). > >>> > >>>The whole business of locking performance counter for exclusive per process > >>>access is a big NO. Which leads me to the questionable usefullness of u
[PATCH] drm/radeon: remove discardable flag from radeon_gem_object_create
On Mon, Jul 21, 2014 at 7:27 AM, Christian K?nig
wrote:
> From: Christian K?nig
>
> Unused and unimplemented. Also fix specifying the
> kernel flag incorrectly at one occasion.
>
> Signed-off-by: Christian K?nig
Applied to my 3.17 tree.
Alex
> ---
> drivers/gpu/drm/radeon/radeon.h | 2 +-
> drivers/gpu/drm/radeon/radeon_fb.c | 3 +--
> drivers/gpu/drm/radeon/radeon_gem.c | 7 +++
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 6ce80ae..9b6fbda 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -548,7 +548,7 @@ int radeon_gem_init(struct radeon_device *rdev);
> void radeon_gem_fini(struct radeon_device *rdev);
> int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
> int alignment, int initial_domain,
> - u32 flags, bool discardable, bool kernel,
> + u32 flags, bool kernel,
> struct drm_gem_object **obj);
>
> int radeon_mode_dumb_create(struct drm_file *file_priv,
> diff --git a/drivers/gpu/drm/radeon/radeon_fb.c
> b/drivers/gpu/drm/radeon/radeon_fb.c
> index 477ea0d..94b0f2a 100644
> --- a/drivers/gpu/drm/radeon/radeon_fb.c
> +++ b/drivers/gpu/drm/radeon/radeon_fb.c
> @@ -127,8 +127,7 @@ static int radeonfb_create_pinned_object(struct
> radeon_fbdev *rfbdev,
> aligned_size = ALIGN(size, PAGE_SIZE);
> ret = radeon_gem_object_create(rdev, aligned_size, 0,
>RADEON_GEM_DOMAIN_VRAM,
> - 0, false, true,
> - &gobj);
> + 0, true, &gobj);
> if (ret) {
> printk(KERN_ERR "failed to allocate framebuffer (%d)\n",
>aligned_size);
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c
> b/drivers/gpu/drm/radeon/radeon_gem.c
> index eafe316..9fb1c06 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -42,7 +42,7 @@ void radeon_gem_object_free(struct drm_gem_object *gobj)
>
> int radeon_gem_object_create(struct radeon_device *rdev, unsigned long size,
> int alignment, int initial_domain,
> - u32 flags, bool discardable, bool kernel,
> + u32 flags, bool kernel,
> struct drm_gem_object **obj)
> {
> struct radeon_bo *robj;
> @@ -255,7 +255,7 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void
> *data,
> args->size = roundup(args->size, PAGE_SIZE);
> r = radeon_gem_object_create(rdev, args->size, args->alignment,
> args->initial_domain, args->flags,
> -false, false, &gobj);
> +false, &gobj);
> if (r) {
> up_read(&rdev->exclusive_lock);
> r = radeon_gem_handle_lockup(rdev, r);
> @@ -570,8 +570,7 @@ int radeon_mode_dumb_create(struct drm_file *file_priv,
>
> r = radeon_gem_object_create(rdev, args->size, 0,
> RADEON_GEM_DOMAIN_VRAM, 0,
> -false, ttm_bo_type_device,
> -&gobj);
> +false, &gobj);
> if (r)
> return -ENOMEM;
>
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 81579] VDPAU: videos slow to a crawl when more than 1 video is playing
https://bugs.freedesktop.org/show_bug.cgi?id=81579 --- Comment #6 from joaoboia at outlook.com --- Created attachment 103186 --> https://bugs.freedesktop.org/attachment.cgi?id=103186&action=edit dmesg -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/b61f9a59/attachment.html>
[PATCH v6 10/14] drm/panel: add S6E3FA0 driver
On Fri, Jul 18, 2014 at 10:49:35AM +0900, YoungJun Cho wrote:
> Hi Thierry,
>
> Thank you a lot for kind comments.
>
> On 07/17/2014 07:36 PM, Thierry Reding wrote:
> > On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
[...]
> > >diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
> > >b/drivers/gpu/drm/panel/panel-s6e3fa0.c
[...]
> > >+static void s6e3fa0_set_maximum_return_packet_size(struct s6e3fa0 *ctx,
> > >+ unsigned int size)
> > >+{
> > >+ struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> > >+ const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> > >+
> > >+ if (ops && ops->transfer) {
> > >+ unsigned char buf[] = {size, 0};
> > >+ struct mipi_dsi_msg msg = {
> > >+ .channel = dsi->channel,
> > >+ .type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
> > >+ .tx_len = sizeof(buf),
> > >+ .tx_buf = buf
> > >+ };
> > >+
> > >+ ops->transfer(dsi->host, &msg);
> > >+ }
> > >+}
> >
> > The Set Maximum Return Packet Size command is a standard command, so
> > please turn that into a generic function exposed by the DSI core.
> >
>
> For this and below standard DCS commands, you want to use generic functions,
> but I have no idea for that.
> Could you explain more detail?
The goal should be to make these standard DCS commands available to all
DSI peripherals, so the implementation should look something like this:
static int mipi_dsi_set_maximum_return_packet_size(struct mipi_dsi_device *dsi,
u16 size)
{
struct mipi_dsi_msg msg;
if (!dsi->ops || !dsi->ops->transfer)
return -ENOSYS;
memset(&msg, 0, sizeof(msg));
msg.channel = dsi->channel;
msg.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE;
msg.tx_len = sizeof(size);
msg.tx_buf = &size;
return dsi->ops->transfer(dsi->host, &msg);
}
The above is somewhat special, since it isn't DCS. For DCS I'd suggest a
common prefix, like so:
enum mipi_dcs_tear_mode {
MIPI_DCS_TEAR_VBLANK,
MIPI_DCS_TEAR_BLANK,
};
static int mipi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
enum mipi_dcs_tear_mode mode)
{
u8 data[2] = { MIPI_DSI_DCS_SET_TEAR_ON, mode };
struct mipi_dsi_msg msg;
if (!dsi->ops || !dsi->ops->transfer)
return -ENOSYS;
memset(&msg, 0, sizeof(msg));
msg.channel = dsi->channel;
msg.type = MIPI_DSI_DCS_SHORT_WRITE_PARAM;
msg.tx_len = sizeof(data);
msg.tx_buf = data;
return dsi->ops->transfer(dsi->host, &msg);
}
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/457df664/attachment.sig>
[PATCH v2 00/25] AMDKFD kernel driver
Am 21.07.2014 09:01, schrieb Daniel Vetter: > On Sun, Jul 20, 2014 at 01:46:53PM -0400, Jerome Glisse wrote: >> On Thu, Jul 17, 2014 at 04:57:25PM +0300, Oded Gabbay wrote: >>> Forgot to cc mailing list on cover letter. Sorry. >>> >>> As a continuation to the existing discussion, here is a v2 patch series >>> restructured with a cleaner history and no totally-different-early-versions >>> of the code. >>> >>> Instead of 83 patches, there are now a total of 25 patches, where 5 of them >>> are modifications to radeon driver and 18 of them include only amdkfd code. >>> There is no code going away or even modified between patches, only added. >>> >>> The driver was renamed from radeon_kfd to amdkfd and moved to reside under >>> drm/radeon/amdkfd. This move was done to emphasize the fact that this driver >>> is an AMD-only driver at this point. Having said that, we do foresee a >>> generic hsa framework being implemented in the future and in that case, we >>> will adjust amdkfd to work within that framework. >>> >>> As the amdkfd driver should support multiple AMD gfx drivers, we want to >>> keep it as a seperate driver from radeon. Therefore, the amdkfd code is >>> contained in its own folder. The amdkfd folder was put under the radeon >>> folder because the only AMD gfx driver in the Linux kernel at this point >>> is the radeon driver. Having said that, we will probably need to move it >>> (maybe to be directly under drm) after we integrate with additional AMD gfx >>> drivers. >>> >>> For people who like to review using git, the v2 patch set is located at: >>> http://cgit.freedesktop.org/~gabbayo/linux/log/?h=kfd-next-3.17-v2 >>> >>> Written by Oded Gabbayh >> So quick comments before i finish going over all patches. There is many >> things that need more documentation espacialy as of right now there is >> no userspace i can go look at. >> >> There few show stopper, biggest one is gpu memory pinning this is a big >> no, that would need serious arguments for any hope of convincing me on >> that side. >> >> It might be better to add a drivers/gpu/drm/amd directory and add common >> stuff there. >> >> Given that this is not intended to be final HSA api AFAICT then i would >> say this far better to avoid the whole kfd module and add ioctl to radeon. >> This would avoid crazy communication btw radeon and kfd. >> >> The whole aperture business needs some serious explanation. Especialy as >> you want to use userspace address there is nothing to prevent userspace >> program from allocating things at address you reserve for lds, scratch, >> ... only sane way would be to move those lds, scratch inside the virtual >> address reserved for kernel (see kernel memory map). >> >> The whole business of locking performance counter for exclusive per process >> access is a big NO. Which leads me to the questionable usefullness of user >> space command ring. I only see issues with that. First and foremost i would >> need to see solid figures that kernel ioctl or syscall has a higher an >> overhead that is measurable in any meaning full way against a simple >> function call. I know the userspace command ring is a big marketing features >> that please ignorant userspace programmer. But really this only brings issues >> and for absolutely not upside afaict. >> >> So i would rather see a very simple ioctl that write the doorbell and might >> do more than that in case of ring/queue overcommit where it would first have >> to wait for a free ring/queue to schedule stuff. This would also allow sane >> implementation of things like performance counter that could be acquire by >> kernel for duration of a job submitted by userspace. While still not optimal >> this would be better that userspace locking. > Quick aside and mostly off the record: In i915 we plan to have the first > implementation exactly as Jerome suggests here: > - New flag at context creationg for svm/seamless-gpgpu contexts. > - New ioctl in i915 for submitting stuff to the hw (through doorbell or >whatever else we want to do). The ring in the ctx would be under the >kernel's control. And looking at the existing Radeon code, that's exactly what we are already doing with the compute queues on CIK as well. We just use the existing command submission interface, cause when you use an IOCTL anyway it's not beneficial any more to do all the complex scheduling and other stuff directly on the hardware. What's mostly missing in the existing module is proper support for accessing the CPU address space through IOMMUv2. Christian. > Of course there's lots of GEM stuff we don't need at all for such > contexts, but there's still lots of shared code. Imo creating a 2nd driver > has too much interface surface and so is a maintainence hell. > > And the ioctl submission gives us flexibility in case the hw doesn't quite > live up to promise (e.g. scheduling, cmd parsing, ...). > -Daniel
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
Hi Boris, On Monday 21 July 2014 11:24:38 Boris BREZILLON wrote: > On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > > On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > > [...] > > > >>> + - atmel,panel: Should contain a phandle with 2 parameters. > >>> + The first cell is a phandle to a DRM panel device > >>> + The second cell encodes the RGB mode, which can take the > >>> following values: > >>> + * 0: RGB444 > >>> + * 1: RGB565 > >>> + * 2: RGB666 > >>> + * 3: RGB888 > >> > >> These are properties of the panel and should be obtained from > >> the panel directly rather than an additional cell in this specifier. > > > > Okay. > > What's the preferred way of doing this ? > > What about defining an rgb-mode property in the panel node. > > There's .bpc in struct drm_display_info, I suspect that it could be > used for this. Alternatively, maybe we could extend the list of color > formats that go into drm_display_info.color_formats? RGB444 is already > covered. > >> > >> I forgot to ask about bpc meaning. If, as I think, it means "bits per > >> color" then it cannot be used to encode RGB565 where green color is > >> encoded on 6 bits and red and blue are encoded on 5 bits. > > > > Yes, I agree that bps is not a good fit for what you need here. > > Okay, then I think we can replace bpc and color_formats by a bus_formats > table containing all supported formats, and use an enum (something > similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h [1]) to list > the available formats. > > As this implies quite a few changes in crtc core and some drm drivers > (nouveau, i915 and radeon), I'd like to be sure this is what both of you > had in mind. I think it is, but just to make sure I understand you correctly, could you just show how the drm_display_info structure would look like ? -- Regards, Laurent Pinchart
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Mon, 21 Jul 2014 10:59:12 +0200 Thierry Reding wrote: > On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: > [...] > > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters. > > > > > > > + The first cell is a phandle to a DRM panel device > > > > > > > + The second cell encodes the RGB mode, which can take the > > > > > > > following values: > > > > > > > + * 0: RGB444 > > > > > > > + * 1: RGB565 > > > > > > > + * 2: RGB666 > > > > > > > + * 3: RGB888 > > > > > > > > > > > > These are properties of the panel and should be obtained from the > > > > > > panel > > > > > > directly rather than an additional cell in this specifier. > > > > > > > > > > Okay. > > > > > What's the preferred way of doing this ? > > > > > What about defining an rgb-mode property in the panel node. > > > > > > > > There's .bpc in struct drm_display_info, I suspect that it could be used > > > > for this. Alternatively, maybe we could extend the list of color formats > > > > that go into drm_display_info.color_formats? RGB444 is already covered. > > > > > > > I forgot to ask about bpc meaning. If, as I think, it means "bits per > > color" then it cannot be used to encode RGB565 where green color is > > encoded on 6 bits and red and blue are encoded on 5 bits. > > Yes, I agree that bps is not a good fit for what you need here. Okay, then I think we can replace bpc and color_formats by a bus_formats table containing all supported formats, and use an enum (something similar to v4l2_mbus_pixelcode defined in v4l2-mediabus.h [1]) to list the available formats. As this implies quite a few changes in crtc core and some drm drivers (nouveau, i915 and radeon), I'd like to be sure this is what both of you had in mind. Best Regards, Boris [1]http://lxr.free-electrons.com/source/include/uapi/linux/v4l2-mediabus.h#L37 -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
[PATCH 00/12] drm/exynos/ipp: image post processing improvements, part three
Hi Inki, On 07/09/2014 04:47 PM, Inki Dae wrote: > On 2014? 07? 03? 22:10, Andrzej Hajda wrote: >> This set of independent patches contains various improvement and fixes >> for exynos_drm ipp framework. >> The patchset is based on exynos-drm-next branch. >> > Did you test ipp module using libdrm? If so, can you share the app? I > would try to test this patch series before merging them. Mainline libdrm > has no any ipp interfaces. I have used ipptest program developed by you :), with minor changes. More advanced tests are on my TODO list, but for this patchset ipptest seems to be OK - the patches are mainly cleanup patches. Regards Andrzej > > Thanks, > Inki Dae > >> Regards >> Andrzej >> >> >> Andrzej Hajda (12): >> drm/exynos/ipp: remove type casting >> drm/exynos/ipp: remove unused field from exynos_drm_ipp_private >> drm/exynos/ipp: remove struct exynos_drm_ipp_private >> drm/exynos/ipp: correct address type >> drm/exynos/ipp: remove temporary variable >> drm/exynos/ipp: remove incorrect checks of list_first_entry result >> drm/exynos/ipp: simplify memory check function >> drm/exynos/ipp: remove useless registration checks >> drm/exynos/ipp: simplify ipp_find_obj >> drm/exynos/ipp: remove redundant messages >> drm/exynos/ipp: simplify ipp_create_id >> drm/exynos/ipp: simplify ipp_find_driver >> >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 7 +- >> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 259 >> +--- >> drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +- >> 3 files changed, 73 insertions(+), 197 deletions(-) >> >
[PATCH v6 10/14] drm/panel: add S6E3FA0 driver
On Mon, Jul 21, 2014 at 10:56:08AM +0200, Andrzej Hajda wrote:
> On 07/18/2014 03:49 AM, YoungJun Cho wrote:
> > Hi Thierry,
> >
> > Thank you a lot for kind comments.
> >
> > On 07/17/2014 07:36 PM, Thierry Reding wrote:
> >> On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
> >> [...]
> >>> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
> >>> b/drivers/gpu/drm/panel/panel-s6e3fa0.c
> >> [...]
> >>> +/* Manufacturer Command Set */
> >>> +#define MCS_GLOBAL_PARAMETER 0xb0
> >>> +#define MCS_AID 0xb2
> >>> +#define MCS_ELVSSOPT 0xb6
> >>> +#define MCS_TEMPERATURE_SET 0xb8
> >>> +#define MCS_PENTILE_CTRL 0xc0
> >>> +#define MCS_GAMMA_MODE 0xca
> >>> +#define MCS_VDDM 0xd7
> >>> +#define MCS_ALS 0xe3
> >>> +#define MCS_ERR_FG 0xed
> >>> +#define MCS_KEY_LEV1 0xf0
> >>> +#define MCS_GAMMA_UPDATE 0xf7
> >>> +#define MCS_KEY_LEV2 0xfc
> >>> +#define MCS_RE 0xfe
> >>> +#define MCS_TOUT2_HSYNC 0xff
> >>> +
> >>> +/* Content Adaptive Brightness Control */
> >>> +#define DCS_WRITE_CABC 0x55
> >> Is this not a manufacturer specific command? I couldn't find it in the
> >> DSI or DCS specifications, but it sounds like something standard (also
> >> indicated by the DCS_ prefix). Can you point out the specification for
> >> this?
> >>
> > Andrzej commented before and decided it as DCS one because if the value
> > is less than 0xb0, it is DCS one and the others are MCS one.
> > But still I'm not sure it is correct.
> I would not say 'decided'. I have just stated that according to DCS
> specification
> it should be DCS command (below 0xb0), but it is not present in mipi dcs
> specs.
> On the other side many panels have it [1]:
>
> [1]:
> https://www.google.com/search?q=%22Write+Content+Adaptive+Brightness+Control%22
Yeah, my search yielded similar results. I wonder if this is perhaps
part of a draft future specification. I'll try to ask around some more
if anybody knows what the status of this is.
> >>> +static void s6e3fa0_read_mtp_id(struct s6e3fa0 *ctx)
> >>> +{
> >>> + unsigned char id[MTP_ID_LEN];
> >>> + int ret;
> >>> +
> >>> + s6e3fa0_set_maximum_return_packet_size(ctx, MTP_ID_LEN);
> >>> + ret = s6e3fa0_dcs_read(ctx, MIPI_DCS_GET_DISPLAY_ID, id, MTP_ID_LEN);
> >> This also looks like a standard DCS command. I can't find it in either
> >> v1.01 nor v1.02 of the specification, though. Do you know where it's
> >> specified?
> >>
> > Yes, I also can't find it in DCS specification and there is no special
> > description in panel datasheet.
> > But as it is declared in "include/video/mipi_display.h", so I think it
> > is a standard one.
>
> On page 9 of the "Introduction of MIPI by Renesas" [2] there is info
> it is a part of "Nokia I/F command list", I guess it is kind of alpha
> version of MIPI DCS.
>
> [2]: http://wenku.baidu.com/view/658fab68af1ffc4ffe47acbe.html
That link is the only one which contains "Nokia I/F command list" on the
internet (that is, according to Google). But since this is already part
of the mipi_display.h header file we may as well use it.
I wonder if perhaps the READ_DDB_START and READ_DDB_CONTINUE commands
could be used as a replacement for this display ID.
Adding Guennadi, Tomi, Paul and Imre on Cc since they were involved with
the original patch that added mipi_display.h. Perhaps they remember what
the origin of this command is.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/2254a498/attachment.sig>
[Bug 81579] VDPAU: videos slow to a crawl when more than 1 video is playing
https://bugs.freedesktop.org/show_bug.cgi?id=81579 --- Comment #5 from Christian K?nig --- Please provide your full dmesg with dpm enabled, but that sounds like your card just doesn't have enough power for two 1080p streams. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/3d08f353/attachment-0001.html>
[Bug 74335] [UVD] vdpau has terrible performance on radeonsi (HD 7950)
https://bugs.freedesktop.org/show_bug.cgi?id=74335 Christian K?nig changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #4 from Christian K?nig --- UVD IRQs weren't working correctly on SI. Bug is fixed upstream by new kernel release. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/9401cda5/attachment.html>
[Bug 81579] VDPAU: videos slow to a crawl when more than 1 video is playing
https://bugs.freedesktop.org/show_bug.cgi?id=81579 --- Comment #4 from joaoboia at outlook.com --- (In reply to comment #3) > That just sounds like your hardware is to slow to handle two 1080p videos. > What hardware are you using? > > Apart from that falling back to software decoding if the hardware decoding > capacity is used up is job of the media player application. Denying decoding > if we already have a stream playing would just result in an black screen. It's a HD4770, it would be UVD 2.2 I think. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/0cb2ba5a/attachment.html>
[RESEND PATCH v3 06/11] drm: add DT bindings documentation for atmel-hlcdc-dc driver
On Fri, Jul 18, 2014 at 05:43:34PM +0200, Boris BREZILLON wrote: [...] > > > > > > + - atmel,panel: Should contain a phandle with 2 parameters. > > > > > > + The first cell is a phandle to a DRM panel device > > > > > > + The second cell encodes the RGB mode, which can take the > > > > > > following values: > > > > > > + * 0: RGB444 > > > > > > + * 1: RGB565 > > > > > > + * 2: RGB666 > > > > > > + * 3: RGB888 > > > > > > > > > > These are properties of the panel and should be obtained from the > > > > > panel > > > > > directly rather than an additional cell in this specifier. > > > > > > > > Okay. > > > > What's the preferred way of doing this ? > > > > What about defining an rgb-mode property in the panel node. > > > > > > There's .bpc in struct drm_display_info, I suspect that it could be used > > > for this. Alternatively, maybe we could extend the list of color formats > > > that go into drm_display_info.color_formats? RGB444 is already covered. > > > > I forgot to ask about bpc meaning. If, as I think, it means "bits per > color" then it cannot be used to encode RGB565 where green color is > encoded on 6 bits and red and blue are encoded on 5 bits. Yes, I agree that bps is not a good fit for what you need here. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/afb8110f/attachment.sig>
[PATCH v6 10/14] drm/panel: add S6E3FA0 driver
On 07/18/2014 03:49 AM, YoungJun Cho wrote:
> Hi Thierry,
>
> Thank you a lot for kind comments.
>
> On 07/17/2014 07:36 PM, Thierry Reding wrote:
>> On Thu, Jul 17, 2014 at 06:01:25PM +0900, YoungJun Cho wrote:
>> [...]
>>> diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c
>>> b/drivers/gpu/drm/panel/panel-s6e3fa0.c
>> [...]
>>> +/* Manufacturer Command Set */
>>> +#define MCS_GLOBAL_PARAMETER 0xb0
>>> +#define MCS_AID0xb2
>>> +#define MCS_ELVSSOPT 0xb6
>>> +#define MCS_TEMPERATURE_SET0xb8
>>> +#define MCS_PENTILE_CTRL 0xc0
>>> +#define MCS_GAMMA_MODE 0xca
>>> +#define MCS_VDDM 0xd7
>>> +#define MCS_ALS0xe3
>>> +#define MCS_ERR_FG 0xed
>>> +#define MCS_KEY_LEV1 0xf0
>>> +#define MCS_GAMMA_UPDATE 0xf7
>>> +#define MCS_KEY_LEV2 0xfc
>>> +#define MCS_RE 0xfe
>>> +#define MCS_TOUT2_HSYNC0xff
>>> +
>>> +/* Content Adaptive Brightness Control */
>>> +#define DCS_WRITE_CABC 0x55
>> Is this not a manufacturer specific command? I couldn't find it in the
>> DSI or DCS specifications, but it sounds like something standard (also
>> indicated by the DCS_ prefix). Can you point out the specification for
>> this?
>>
> Andrzej commented before and decided it as DCS one because if the value
> is less than 0xb0, it is DCS one and the others are MCS one.
> But still I'm not sure it is correct.
I would not say 'decided'. I have just stated that according to DCS
specification
it should be DCS command (below 0xb0), but it is not present in mipi dcs
specs.
On the other side many panels have it [1]:
[1]:
https://www.google.com/search?q=%22Write+Content+Adaptive+Brightness+Control%22
>>> +#define MTP_ID_LEN 3
>>> +#define GAMMA_LEVEL_NUM30
>>> +
>>> +#define DEFAULT_VDDM_VAL 0x15
>>> +
>>> +struct s6e3fa0 {
>>> + struct device *dev;
>>> + struct drm_panelpanel;
>>> +
>>> + struct regulator_bulk_data supplies[2];
>>> + struct gpio_desc*reset_gpio;
>>> + struct videomodevm;
>>> +
>>> + unsigned intpower_on_delay;
>>> + unsigned intreset_delay;
>>> + unsigned intinit_delay;
>>> + unsigned intwidth_mm;
>>> + unsigned intheight_mm;
>>> +
>>> + unsigned char id;
>>> + unsigned char vddm;
>>> + unsigned intbrightness;
>>> +};
>> Please don't use this kind of artificial padding. A simple space will
>> do.
>>
>>> +
>>> +#define panel_to_s6e3fa0(p) container_of(p, struct s6e3fa0, panel)
>> Please turn this into a function so we can get proper type checking.
>>
>>> +
>>> +/* VDD Memory Lookup Table contains pairs of {ReadValue, WriteValue} */
>>> +static const unsigned char s6e3fa0_vddm_lut[][2] = {
>>> + {0x00, 0x0d}, {0x01, 0x0d}, {0x02, 0x0e}, {0x03, 0x0f}, {0x04, 0x10},
>>> + {0x05, 0x11}, {0x06, 0x12}, {0x07, 0x13}, {0x08, 0x14}, {0x09, 0x15},
>>> + {0x0a, 0x16}, {0x0b, 0x17}, {0x0c, 0x18}, {0x0d, 0x19}, {0x0e, 0x1a},
>>> + {0x0f, 0x1b}, {0x10, 0x1c}, {0x11, 0x1d}, {0x12, 0x1e}, {0x13, 0x1f},
>>> + {0x14, 0x20}, {0x15, 0x21}, {0x16, 0x22}, {0x17, 0x23}, {0x18, 0x24},
>>> + {0x19, 0x25}, {0x1a, 0x26}, {0x1b, 0x27}, {0x1c, 0x28}, {0x1d, 0x29},
>>> + {0x1e, 0x2a}, {0x1f, 0x2b}, {0x20, 0x2c}, {0x21, 0x2d}, {0x22, 0x2e},
>>> + {0x23, 0x2f}, {0x24, 0x30}, {0x25, 0x31}, {0x26, 0x32}, {0x27, 0x33},
>>> + {0x28, 0x34}, {0x29, 0x35}, {0x2a, 0x36}, {0x2b, 0x37}, {0x2c, 0x38},
>>> + {0x2d, 0x39}, {0x2e, 0x3a}, {0x2f, 0x3b}, {0x30, 0x3c}, {0x31, 0x3d},
>>> + {0x32, 0x3e}, {0x33, 0x3f}, {0x34, 0x3f}, {0x35, 0x3f}, {0x36, 0x3f},
>>> + {0x37, 0x3f}, {0x38, 0x3f}, {0x39, 0x3f}, {0x3a, 0x3f}, {0x3b, 0x3f},
>>> + {0x3c, 0x3f}, {0x3d, 0x3f}, {0x3e, 0x3f}, {0x3f, 0x3f}, {0x40, 0x0c},
>>> + {0x41, 0x0b}, {0x42, 0x0a}, {0x43, 0x09}, {0x44, 0x08}, {0x45, 0x07},
>>> + {0x46, 0x06}, {0x47, 0x05}, {0x48, 0x04}, {0x49, 0x03}, {0x4a, 0x02},
>>> + {0x4b, 0x01}, {0x4c, 0x40}, {0x4d, 0x41}, {0x4e, 0x42}, {0x4f, 0x43},
>>> + {0x50, 0x44}, {0x51, 0x45}, {0x52, 0x46}, {0x53, 0x47}, {0x54, 0x48},
>>> + {0x55, 0x49}, {0x56, 0x4a}, {0x57, 0x4b}, {0x58, 0x4c}, {0x59, 0x4d},
>>> + {0x5a, 0x4e}, {0x5b, 0x4f}, {0x5c, 0x50}, {0x5d, 0x51}, {0x5e, 0x52},
>>> + {0x5f, 0x53}, {0x60, 0x54}, {0x61, 0x55}, {0x62, 0x56}, {0x63, 0x57},
>>> + {0x64, 0x58}, {0x65, 0x59}, {0x66, 0x5a}, {0x67, 0x5b}, {0x68, 0x5c},
>>> + {0x69, 0x5d}, {0x6a, 0x5e}, {0x6b, 0x5f}, {0x6c, 0x60}, {0x6d, 0x61},
>>> + {0x6e, 0x62}, {0x6f, 0x63}, {0x70, 0x64}, {0x71, 0x65}, {0x72, 0x66},
>>> + {0x73, 0x67}, {0x74, 0x68}, {0x75, 0x69}, {0x76, 0x6a}, {0x77, 0x6b},
>>> + {0x78, 0x6c}, {0x79, 0x6d}, {0x7a, 0x6e}, {0x7b, 0x6f}, {0x7c, 0x70},
>>> + {0x7d, 0x71}, {0x7e, 0x72}, {0x7f, 0x73},
>>> +};
>> What's this used for?
>>
[PATCH] drm/dp: Staticize a couple of DP utility functions
On Mon, Jul 21, 2014 at 7:23 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> sparse complains about these functions missing a prototype, but looking
> closer they aren't (and likely not supposed to be) used outside of this
> source file, so they can be made static.
>
> Signed-off-by: Thierry Reding
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 30 +++---
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 369d6c49145b..c9a772554308 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1482,10 +1482,10 @@ static int drm_dp_send_enum_path_resources(struct
> drm_dp_mst_topology_mgr *mgr,
> return 0;
> }
>
> -int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> - struct drm_dp_mst_port *port,
> - int id,
> - int pbn)
> +static int drm_dp_payload_send_msg(struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_mst_port *port,
> + int id,
> + int pbn)
> {
> struct drm_dp_sideband_msg_tx *txmsg;
> struct drm_dp_mst_branch *mstb;
> @@ -1536,10 +1536,10 @@ static int drm_dp_create_payload_step1(struct
> drm_dp_mst_topology_mgr *mgr,
> return 0;
> }
>
> -int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
> - struct drm_dp_mst_port *port,
> - int id,
> - struct drm_dp_payload *payload)
> +static int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_mst_port *port,
> + int id,
> + struct drm_dp_payload *payload)
> {
> int ret;
> ret = drm_dp_payload_send_msg(mgr, port, id, port->vcpi.pbn);
> @@ -1549,10 +1549,10 @@ int drm_dp_create_payload_step2(struct
> drm_dp_mst_topology_mgr *mgr,
> return ret;
> }
>
> -int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
> -struct drm_dp_mst_port *port,
> -int id,
> -struct drm_dp_payload *payload)
> +static int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_mst_port *port,
> + int id,
> + struct drm_dp_payload *payload)
> {
> DRM_DEBUG_KMS("\n");
> /* its okay for these to fail */
> @@ -1565,9 +1565,9 @@ int drm_dp_destroy_payload_step1(struct
> drm_dp_mst_topology_mgr *mgr,
> return 0;
> }
>
> -int drm_dp_destroy_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
> -int id,
> -struct drm_dp_payload *payload)
> +static int drm_dp_destroy_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
> + int id,
> + struct drm_dp_payload *payload)
> {
> payload->payload_state = 0;
> return 0;
> --
> 2.0.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: Fix a few sparse warnings
On Mon, Jul 21, 2014 at 7:15 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> The final parameter to ttm_bo_reserve() is a pointer, therefore callers
> should use NULL instead of 0.
>
> Fixes a bunch of sparse warnings of this type:
>
> warning: Using plain integer as NULL pointer
>
> Signed-off-by: Thierry Reding
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/ast/ast_drv.h| 2 +-
> drivers/gpu/drm/bochs/bochs_fbdev.c | 2 +-
> drivers/gpu/drm/bochs/bochs_kms.c| 4 ++--
> drivers/gpu/drm/cirrus/cirrus_drv.h | 2 +-
> drivers/gpu/drm/mgag200/mgag200_drv.h| 2 +-
> drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +++---
> drivers/gpu/drm/nouveau/nouveau_gem.c| 4 ++--
> drivers/gpu/drm/qxl/qxl_object.h | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_dmabuf.c | 6 +++---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 2 +-
> 12 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 5d6a87573c33..957d4fabf1e1 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -362,7 +362,7 @@ static inline int ast_bo_reserve(struct ast_bo *bo, bool
> no_wait)
> {
> int ret;
>
> - ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
> if (ret) {
> if (ret != -ERESTARTSYS && ret != -EBUSY)
> DRM_ERROR("reserve failed %p\n", bo);
> diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c
> b/drivers/gpu/drm/bochs/bochs_fbdev.c
> index 19cf3e9413b6..fe95d31cd110 100644
> --- a/drivers/gpu/drm/bochs/bochs_fbdev.c
> +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
> @@ -72,7 +72,7 @@ static int bochsfb_create(struct drm_fb_helper *helper,
>
> bo = gem_to_bochs_bo(gobj);
>
> - ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> b/drivers/gpu/drm/bochs/bochs_kms.c
> index dcf2e55f4ae9..9abd2312f5bd 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -53,7 +53,7 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc,
> int x, int y,
> if (old_fb) {
> bochs_fb = to_bochs_framebuffer(old_fb);
> bo = gem_to_bochs_bo(bochs_fb->obj);
> - ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
> if (ret) {
> DRM_ERROR("failed to reserve old_fb bo\n");
> } else {
> @@ -67,7 +67,7 @@ static int bochs_crtc_mode_set_base(struct drm_crtc *crtc,
> int x, int y,
>
> bochs_fb = to_bochs_framebuffer(crtc->primary->fb);
> bo = gem_to_bochs_bo(bochs_fb->obj);
> - ret = ttm_bo_reserve(&bo->bo, true, false, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, false, false, NULL);
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
> b/drivers/gpu/drm/cirrus/cirrus_drv.h
> index 117d3eca5e37..401c890b6c6a 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.h
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
> @@ -241,7 +241,7 @@ static inline int cirrus_bo_reserve(struct cirrus_bo *bo,
> bool no_wait)
> {
> int ret;
>
> - ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
> if (ret) {
> if (ret != -ERESTARTSYS && ret != -EBUSY)
> DRM_ERROR("reserve failed %p\n", bo);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> index cf11ee68a6d9..80de23d9b9c9 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> @@ -280,7 +280,7 @@ static inline int mgag200_bo_reserve(struct mgag200_bo
> *bo, bool no_wait)
> {
> int ret;
>
> - ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, 0);
> + ret = ttm_bo_reserve(&bo->bo, true, no_wait, false, NULL);
> if (ret) {
> if (ret != -ERESTARTSYS && ret != -EBUSY)
> DRM_ERROR("reserve failed %p\n", bo);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index f00ae18003f1..58e49dcf5bd8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -310,7 +310,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t memtype)
> struct ttm_buffer_object *bo = &nvbo->bo;
> int ret;
>
> - ret = ttm_bo_reserve(bo, false, false, false, 0);
> + ret = ttm_bo_re
[RESEND PATCH V5 08/12] drm/bridge: ptn3460: Support bridge chaining
On Fri, Jul 18, 2014 at 02:13:54AM +0530, Ajay Kumar wrote: [...] > Also, remove the drm_connector implementation from ptn3460, > since the same is implemented using panel_binder. I think that's a step backwards. In fact I think the panel-bridge binder driver shouldn't be needed at all. At least not for now. We have a very limited number of bridge drivers, so it shouldn't hurt at this stage to implement the connector instantiation within each driver. Once we have more bridge drivers, and therefore a better understanding of what they need, we can always add something like the panel-binder (though I think it should be library code, similar to the drm_encoder_helper_add() API, rather than this kind of self-contained object). Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/b240f88d/attachment-0001.sig>
[RESEND PATCH V5 03/12] drm/exynos: dp: modify driver to support drm_panel
On Fri, Jul 18, 2014 at 02:13:49AM +0530, Ajay Kumar wrote:
> Add drm_panel controls to support powerup/down of the
> eDP panel, if one is present at the sink side.
>
> Signed-off-by: Ajay Kumar
> ---
> .../devicetree/bindings/video/exynos_dp.txt|2 +
> drivers/gpu/drm/exynos/Kconfig |1 +
> drivers/gpu/drm/exynos/exynos_dp_core.c| 45
>
> drivers/gpu/drm/exynos/exynos_dp_core.h|2 +
> 4 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt
> b/Documentation/devicetree/bindings/video/exynos_dp.txt
> index 53dbccf..c029a09 100644
> --- a/Documentation/devicetree/bindings/video/exynos_dp.txt
> +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
> @@ -51,6 +51,8 @@ Required properties for dp-controller:
> LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
> - display-timings: timings for the connected panel as described by
> Documentation/devicetree/bindings/video/display-timing.txt
> + -edp-panel:
> + phandle for the edp/lvds drm_panel node.
There's really no need for the edp- prefix here. Also please don't use
drm_panel in the binding description since it makes no sense outside of
DRM (and bindings need to be OS agnostic).
Also: "eDP" and "LVDS"
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
> b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index a94b114..b3d0d9b 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -28,6 +28,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "exynos_drm_drv.h"
> @@ -1029,6 +1030,9 @@ static int exynos_dp_create_connector(struct
> exynos_drm_display *display,
> drm_connector_register(connector);
> drm_mode_connector_attach_encoder(connector, encoder);
>
> + if (dp->edp_panel)
> + drm_panel_attach(dp->edp_panel, &dp->connector);
This function can fail, so you really need to do some error-checking
here.
> @@ -1063,11 +1067,13 @@ static void exynos_dp_poweron(struct exynos_dp_device
> *dp)
> if (dp->dpms_mode == DRM_MODE_DPMS_ON)
> return;
>
> + drm_panel_prepare(dp->edp_panel);
> clk_prepare_enable(dp->clock);
> exynos_dp_phy_init(dp);
> exynos_dp_init_dp(dp);
> enable_irq(dp->irq);
> exynos_dp_setup(dp);
> + drm_panel_enable(dp->edp_panel);
These two as well. clk_prepare_enable() can also fail by the way.
exynos_dp_init_dp() can also fail theoretically (it returns int) but
never returns anything other than 0, so it could just as well return
void.
> @@ -1075,10 +1081,12 @@ static void exynos_dp_poweroff(struct
> exynos_dp_device *dp)
> if (dp->dpms_mode != DRM_MODE_DPMS_ON)
> return;
>
> + drm_panel_disable(dp->edp_panel);
> disable_irq(dp->irq);
> flush_work(&dp->hotplug_work);
> exynos_dp_phy_exit(dp);
> clk_disable_unprepare(dp->clock);
> + drm_panel_unprepare(dp->edp_panel);
> }
And these two also.
> static void exynos_dp_dpms(struct exynos_drm_display *display, int mode)
> @@ -1209,8 +1217,17 @@ err:
> static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp)
> {
> int ret;
> + struct device_node *videomode_parent;
> +
> + /* Receive video timing info from panel node
> + * if there is a panel node
> + */
> + if (dp->panel_node)
> + videomode_parent = dp->panel_node;
> + else
> + videomode_parent = dp->dev->of_node;
>
> - ret = of_get_videomode(dp->dev->of_node, &dp->panel.vm,
> + ret = of_get_videomode(videomode_parent, &dp->panel.vm,
You shouldn't be grabbing the video timings from some random node like
this. Instead you should use the DRM panel's .get_modes() function to
query the supported modes. The panel may not (in fact, should not, at
least under most circumstances) define video timings in the device tree.
> @@ -1341,6 +1353,21 @@ static int exynos_dp_probe(struct platform_device
> *pdev)
> if (ret)
> return ret;
>
> + dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
> + GFP_KERNEL);
> + if (!dp)
> + return -ENOMEM;
> +
> + dp->panel_node = of_parse_phandle(dev->of_node, "edp-panel", 0);
There should be no need to store the struct device_node * obtained here
in the context like this.
> + if
[PATCH] drm/radeon: let's use GB for vm_size
Am 18.07.2014 19:06, schrieb Alex Deucher:
> On Fri, Jul 18, 2014 at 12:10 PM, Alex Deucher
> wrote:
>> On Fri, Jul 18, 2014 at 7:56 AM, Christian K?nig
>> wrote:
>>> From: Christian K?nig
>>>
>>> VM sizes smaller than 1GB doesn't make much sense anyway.
>>>
>>> Signed-off-by: Christian K?nig
>> Applied to my 3.16 tree.
> Actually there was a small typo. I applied the attached version instead.
Still looks good to me.
Thanks for getting it into 3.16,
Christian.
> Alex
>
>> Alex
>>
>>> ---
>>> drivers/gpu/drm/radeon/radeon_device.c | 14 +++---
>>> drivers/gpu/drm/radeon/radeon_drv.c| 4 ++--
>>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
>>> b/drivers/gpu/drm/radeon/radeon_device.c
>>> index 03686fa..a8537d7 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>>> @@ -1056,22 +1056,22 @@ static void radeon_check_arguments(struct
>>> radeon_device *rdev)
>>> if (!radeon_check_pot_argument(radeon_vm_size)) {
>>> dev_warn(rdev->dev, "VM size (%d) must be a power of 2\n",
>>> radeon_vm_size);
>>> - radeon_vm_size = 4096;
>>> + radeon_vm_size = 4;
>>> }
>>>
>>> - if (radeon_vm_size < 4) {
>>> - dev_warn(rdev->dev, "VM size (%d) to small, min is 4MB\n",
>>> + if (radeon_vm_size < 1) {
>>> + dev_warn(rdev->dev, "VM size (%d) to small, min is 1GB\n",
>>> radeon_vm_size);
>>> - radeon_vm_size = 4096;
>>> + radeon_vm_size = 4;
>>> }
>>>
>>> /*
>>> * Max GPUVM size for Cayman, SI and CI are 40 bits.
>>> */
>>> - if (radeon_vm_size > 1024*1024) {
>>> + if (radeon_vm_size > 1024) {
>>> dev_warn(rdev->dev, "VM size (%d) to large, max is 1TB\n",
>>> radeon_vm_size);
>>> - radeon_vm_size = 4096;
>>> + radeon_vm_size = 4;
>>> }
>>>
>>> /* defines number of bits in page table versus page directory,
>>> @@ -1238,7 +1238,7 @@ int radeon_device_init(struct radeon_device *rdev,
>>> /* Adjust VM size here.
>>> * Max GPUVM size for cayman+ is 40 bits.
>>> */
>>> - rdev->vm_manager.max_pfn = radeon_vm_size << 8;
>>> + rdev->vm_manager.max_pfn = radeon_vm_size << 18;
>>>
>>> /* Set asic functions */
>>> r = radeon_asic_init(rdev);
>>> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
>>> b/drivers/gpu/drm/radeon/radeon_drv.c
>>> index cb14213..e9e3610 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_drv.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
>>> @@ -173,7 +173,7 @@ int radeon_dpm = -1;
>>> int radeon_aspm = -1;
>>> int radeon_runtime_pm = -1;
>>> int radeon_hard_reset = 0;
>>> -int radeon_vm_size = 4096;
>>> +int radeon_vm_size = 4;
>>> int radeon_vm_block_size = 9;
>>> int radeon_deep_color = 0;
>>>
>>> @@ -243,7 +243,7 @@ module_param_named(runpm, radeon_runtime_pm, int, 0444);
>>> MODULE_PARM_DESC(hard_reset, "PCI config reset (1 = force enable, 0 =
>>> disable (default))");
>>> module_param_named(hard_reset, radeon_hard_reset, int, 0444);
>>>
>>> -MODULE_PARM_DESC(vm_size, "VM address space size in megabytes (default
>>> 4GB)");
>>> +MODULE_PARM_DESC(vm_size, "VM address space size in gigabytes (default
>>> 4GB)");
>>> module_param_named(vm_size, radeon_vm_size, int, 0444);
>>>
>>> MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default 9)");
>>> --
>>> 1.9.1
>>>
[PATCH] drm/radeon: adjust default radeon_vm_block_size
Am 19.07.2014 14:37, schrieb Grigori Goronzy:
> On 19.07.2014 13:57, Christian K?nig wrote:
>> Yeah, I'm still playing a bit with this.
>>
>> You need to consider the page directory size as well. If we have 4GB
>> address space (32bits) and 12bits in the page, 12bits in the page tables
>> then there are only 8bits for the page directory, right?
>>
>> Now 8bits for the page directory means we have 256 entries with 8bytes
>> for each entry that makes 2048 bytes for the page directory. But since
>> we allocate 4096 bytes for the page directory anyway we could support
>> 8GB address space as well.
>>
> Yes, that's right... good idea. Larger VM space should also help a bit
> with fragmentation under memory pressure, right?
Only when the application really needs a lot of RAM. Twice as much
address space than memory should be perfectly sufficient in most cases.
>
>> How about the v2 I've just send out to the list? It also adjusts the
>> vm_size to a default of 8GB and let you get vm_block_sizes according to
>> the following table:
>>
> Looks good, I think.
Does that mean that I have you're rb on the v2 of the patch? A bit of
testing might be a good idea as well, cause I just hacked together the
patch in a bit spare time.
If all is well CC Alex as well, so he can replace the original patch
with the v2 in hist 3.17-wip branch.
Regards,
Christian.
>
>> vm_size vm_block_size
>> 1GB=9
>> 2GB=10
>> 4GB=11
>> 8GB=12
>> 16GB =12
>> 32GB =13
>> 64GB =13
>> 128GB=14
>> 256GB=14
>> 512GB=15
>> 1TB=15
>>
>> Regards,
>> Christian.
>>
>> Am 18.07.2014 22:03, schrieb Grigori Goronzy:
>>> On 18.07.2014 11:38, Christian K?nig wrote:
From: Christian K?nig
Signed-off-by: Christian K?nig
---
drivers/gpu/drm/radeon/radeon_device.c | 6 +-
drivers/gpu/drm/radeon/radeon_drv.c| 4 ++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 03686fa..a2960db 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1077,7 +1077,11 @@ static void radeon_check_arguments(struct
radeon_device *rdev)
/* defines number of bits in page table versus page directory,
* a page is 4KB so we have 12 bits offset, minimum 9 bits in the
* page table and the remaining bits are in the page directory */
-if (radeon_vm_block_size < 9) {
+if (radeon_vm_block_size == -1) {
+radeon_vm_block_size = (ilog2(radeon_vm_size) + 10) / 2;
+radeon_vm_block_size = max(radeon_vm_block_size, 9);
+
+} else if (radeon_vm_block_size < 9) {
>>> Hm, that automatic calculation seems strange. Is there any particular
>>> reason why you haven chosen this? This will use vm_block_size=11 for
>>> 4096 MB VM space. I have used vm_block_size=12 with good results. This
>>> manages 16 MB of VM space per page table, which seems reasonable. I also
>>> see a noticeable CPU overhead reduction between 11 and 12.
>>>
>>> Grigori
>>>
dev_warn(rdev->dev, "VM page table size (%d) to small\n",
radeon_vm_block_size);
radeon_vm_block_size = 9;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index cb14213..fef5320 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -174,7 +174,7 @@ int radeon_aspm = -1;
int radeon_runtime_pm = -1;
int radeon_hard_reset = 0;
int radeon_vm_size = 4096;
-int radeon_vm_block_size = 9;
+int radeon_vm_block_size = -1;
int radeon_deep_color = 0;
MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch
registers");
@@ -246,7 +246,7 @@ module_param_named(hard_reset, radeon_hard_reset,
int, 0444);
MODULE_PARM_DESC(vm_size, "VM address space size in megabytes
(default 4GB)");
module_param_named(vm_size, radeon_vm_size, int, 0444);
-MODULE_PARM_DESC(vm_block_size, "VM page table size in bits
(default 9)");
+MODULE_PARM_DESC(vm_block_size, "VM page table size in bits (default
depending on vm_size)");
module_param_named(vm_block_size, radeon_vm_block_size, int, 0444);
MODULE_PARM_DESC(deep_color, "Deep Color support (1 = enable, 0 =
disable (default))");
>
[PATCH 0/5] radeon: Write-combined CPU mappings of BOs in GTT
Am 19.07.2014 03:15, schrieb Michel D?nzer: > On 19.07.2014 00:47, Christian K?nig wrote: >> Am 18.07.2014 05:07, schrieb Michel D?nzer: > [PATCH 5/5] drm/radeon: Use VRAM for indirect buffers on >= SI I'm still not very keen with this change since I still don't understand the reason why it's faster than with GTT. Definitely needs more testing on a wider range of systems. >>> Sure. If anyone wants to give this patch a spin and see if they can >>> measure any performance difference, good or bad, that would be >>> interesting. >>> Maybe limit it to APUs for now? >>> But IIRC, CPU writes to VRAM vs. write-combined GTT are actually an even >>> bigger win with dedicated GPUs than with the Kaveri built-in GPU on my >>> system. I suspect it may depend on the bandwidth available for PCIe vs. >>> system memory though. >> I've made a few tests today with the kernel part of the patches running >> Xonotic on Ultra in 1920 x 1080. >> >> Without any patches I get around ~47.0fps on average with my dedicated >> HD7870. >> >> Adding only "drm/radeon: Use write-combined CPU mappings of rings and >> IBs on >= SI" and that goes down to ~45.3fps. >> >> Adding on to off that "drm/radeon: Use VRAM for indirect buffers on >= >> SI" and the frame rate goes down to ~27.74fps. > Hmm, looks like I'll need to do more benchmarking of 3D workloads as well. > > Alex, given those numbers, it's probably best if you remove the "Use > write-combined CPU mappings of rings and IBs on >= SI" change from your > tree as well for now. I wouldn't go as far as reverting the patch. It just needs a bit more fine tuning and that can happen in the 3.17rc cycle. My tests clearly show that we still can use USWC for the ring buffer on SI and probably earlier chips as well. The performance drop comes from reading the IB content for command stream validation on SI. Putting the IB into VRAM still doesn't seems to be a good idea on dedicated cards, but it might actually make sense on APUs. Regards, Christian.
[RESEND PATCH V5 03/12] drm/exynos: dp: modify driver to support drm_panel
On Fri, Jul 18, 2014 at 02:13:49AM +0530, Ajay Kumar wrote: [...] > diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig [...] > depends on DRM_EXYNOS_FIMD && ARCH_EXYNOS && (DRM_PTN3460=n || > DRM_PTN3460=y || DRM_PTN3460=DRM_EXYNOS) One of the reasons the current way of implementing bridges is that it doesn't scale, as shown by this ridiculous dependency. In patch [RESEND PATCH V5 12/12] drm/exynos: Add ps8622 lvds bridge discovery to DP driver you add support for another type of bridge but it doesn't update this dependency. I suspect that in order for this to work properly you'll need to extend the dependency like so (rewritten to make it somewhat cleaner): depends on DRM_EXYNOS_FIMD && ARCH_EXYNOS depends on DRM_PTN3460=n || DRM_PTN3460=y || DRM_PTN3460=DRM_EXYNOS depends on DRM_PS8622=n || DRM_PS8622=y || DRM_PS8622=DRM_EXYNOS And you'll need one more of these lines for each new bridge chip that you want to support. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/65f1e4f2/attachment-0001.sig>
[PATCH V5 05/12] Documentation: Add DT bindings for panel-lvds driver
t; + Delay in ms needed for the panel LCD unit to > >> > + powerup completely. > >> > + ex: delay needed till eDP panel throws HPD. > >> > + delay needed so that we cans tart reading > >> > edid. > > > > If the panel signals HPD then we don't need this delay at all and we > > should just wait for HPD instead. > Not always for HPD, we need to wait for EDID module as well. I always thought that HPD signalled readiness from the panel to provide EDID, too. Are there really panels that need additional delays after HPD has been signalled until the EDID can be read? Are there maybe other ways to detect that EDID can't be read? > >> > + -panel-enable-delay: > >> > + delay value in ms required for panel_enable process > >> > + Delay in ms needed for the panel backlight/LED > >> > unit > >> > + to powerup, and delay needed between > >> > video_enable and > >> > + backlight_enable. > >> > + -panel-disable-delay: > >> > + delay value in ms required for panel_disable process > >> > + Delay in ms needed for the panel backlight/LED > >> > unit > >> > + powerdown, and delay needed between > >> > backlight_disable > >> > + and video_disable. > >> > + -panel-unprepare-delay: > >> > + delay value in ms required for panel_post_disable process > >> > + Delay in ms needed for the panel LCD unit to > >> > + to powerdown completely, and the minimum delay > >> > needed > >> > + before powering it on again. > > > > These delays are all panel specific and they don't vary per board, so > > they shouldn't go into the device tree at all. > But, fetching them from device tree would allow us to support all lvds > panels in this single driver. You can do that even without having this data in device tree. My point is that this is redundant information once you know the panel's specific compatible value. Therefore there shouldn't be a need to list this in device tree again. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140721/101f9956/attachment.sig>
