Packard Bell EasyNote LV need i915.invert_brightness=1
[Petter Reinholdtsen 2013-06-11] > Sure. This is my first try, so I hope I got it right. Does the silence mean I got the kernel patch formatting right, or that I should try again? http://lists.freedesktop.org/archives/dri-devel/2013-June/039787.html > contain the complete patch with description etc. -- Happy hacking Petter Reinholdtsen
[PATCH] drm: Added SDP and VSC structures for handling PSR for eDP
From: Shobhit Kumar
SDP header and SDP VSC header as per eDP 1.3 spec, section 3.5,
chapter "PSR Secondary Data Package Support".
v2: Modified and corrected the structures to be more in line for
kernel coding guidelines and rebased the code on Paulo's DP patchset
v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
v4: moving them to include/drm/drm_dp_helper.h and also already
icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
for PSR at once at drm_dp_helper.h
v5: Fix SDP VSC header and identation by (Paulo Zanoni) and
remove i915 from title (Daniel Vetter)
v6: Fix spec version and move comments from code to commit message
since numbers might change in the future (by Paulo Zanoni).
CC: Paulo Zanoni
Reviewed-by: Paulo Zanoni
Signed-off-by: Sateesh Kavuri
Signed-off-by: Shobhit Kumar
Signed-off-by: Rodrigo Vivi
---
include/drm/drm_dp_helper.h | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index e8e1417..ae8dbfb 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -342,13 +342,42 @@ u8 drm_dp_get_adjust_request_voltage(u8
link_status[DP_LINK_STATUS_SIZE],
u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
int lane);
-#define DP_RECEIVER_CAP_SIZE 0xf
+#define DP_RECEIVER_CAP_SIZE 0xf
+#define EDP_PSR_RECEIVER_CAP_SIZE 2
+
void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
u8 drm_dp_link_rate_to_bw_code(int link_rate);
int drm_dp_bw_code_to_link_rate(u8 link_bw);
+struct edp_sdp_header {
+ u8 HB0; /* Secondary Data Packet ID */
+ u8 HB1; /* Secondary Data Packet Type */
+ u8 HB2; /* 7:5 reserved, 4:0 revision number */
+ u8 HB3; /* 7:5 reserved, 4:0 number of valid data bytes */
+} __packed;
+
+#define EDP_SDP_HEADER_REVISION_MASK 0x1F
+#define EDP_SDP_HEADER_VALID_PAYLOAD_BYTES 0x1F
+
+struct edp_vsc_psr {
+ struct edp_sdp_header sdp_header;
+ u8 DB0; /* Stereo Interface */
+ u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid */
+ u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
+ u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
+ u8 DB4; /* CRC value bits 7:0 of the G or Y component */
+ u8 DB5; /* CRC value bits 15:8 of the G or Y component */
+ u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
+ u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
+ u8 DB8_31[24]; /* Reserved */
+} __packed;
+
+#define EDP_VSC_PSR_STATE_ACTIVE (1<<0)
+#define EDP_VSC_PSR_UPDATE_RFB (1<<1)
+#define EDP_VSC_PSR_CRC_VALUES_VALID (1<<2)
+
static inline int
drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
{
--
1.8.1.4
[PATCH 1/2] power: add new interface to return pm_transition state
On Monday, June 24, 2013 04:20:06 PM Shuah Khan wrote: > On 06/22/2013 03:17 PM, Rafael J. Wysocki wrote: > > On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: > >> Add a new interface get_pm_transition() to return pm_transition state. > >> This interface is intended to be used from dev_pm_ops class and type > >> suspend interfaces that call legacy pm ops driver suspend interfaces. > >> Legacy suspend pm_ops take pm_message_t as a parameter. > >> > >> e.g: drm_class_suspend() calls into driver suspend routines > >> via drm_dev->driver->suspend(drm_dev, state). > >> > >> Once drm_class_suspend() is converted to dev_pm_ops, it will no longer > >> have access to pm_transition which it has to pass into driver legacy > >> suspend calls. get_pm_transition() interface addresses this need. > > > > That shouldn't be necessary because each transition has its own callback > > in strict dev_pm_ops. > > > > Thanks, > > Rafael > > > > Yes that is correct that there is no need pass in or know pm_transition > with dev_pm_ops. The issue I am running into is the legacy pm_ops class > suspend/resume routines call. In the example, I mentioned in my > changelog, drm_class_suspend() calls legacy pm_ops and passes in state. > It passes in the state to the driver legacy suspend routine. I have seen > code paths in drivers that differentiate between PM_EVENT_FREEZE, > PM_EVENT_SUSPEND, PM_EVENT_SLEEP etc. > > I considered passing PM_EVENT_SUSPEND to > drm_dev->driver->suspend(drm_dev, state) from drm_class_suspend() which > would eliminate the need for this new interface. However, I am concerned > about breaking driver legacy suspend routines that key of off the state > to execute different code paths for PM_EVENT_FREEZE vs. > PM_EVENT_SUSPEND. Suspend routines get called when state is is > PM_EVENT_FREEZE based on my testing. > > I would rather not add a new interface. Hoping you will have another > idea on how to pass in the state to legacy suspend/resume without adding > this new interface. My thinking is that this new interface is temporary > measure until all of the legacy suspend routines get converted to > dev_pm_ops and at the tile legacy interface gets removed, this new > interface can go away as well. There are multiple bus types with legacy callbacks. PCI and platform for two examples and USB does something similar. Please have a look at these. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.
[PATCH 0/4] IPU DMFC bandwidth allocation fix and cleanups
On Fri, Jun 21, 2013 at 10:57:17AM +0200, Philipp Zabel wrote: > Hi, > > the following patches remove some unused variables, replace some > hard-coded channel numbers with existing descriptive defines, fix > the DMFC bandwidth (or rather: FIFO space) allocation, and update > ipu_page_flip() to immediately set crtc->fb. Acked-by: Sascha Hauer Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> ("drm/i915: create compact dma scatter lists for gem objects") makes
> certain assumptions about the under laying DMA API that are not always
> correct.
>
> On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> I see:
>
> [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
>
> Bit of debugging traced it down to dma_map_sg failing (in
> i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>
> That unfortunately are sizes that the SWIOTLB is incapable of handling -
> the maximum it can handle is a an entry of 512KB of virtual contiguous
> memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>
> Previous to the above mention git commit the SG entries were of 4KB, and
> the code introduced by above git commit squashed the CPU contiguous PFNs
> in one big virtual address provided to DMA API.
>
> This patch is a simple semi-revert - were we emulate the old behavior
> if we detect that SWIOTLB is online. If it is not online then we continue
> on with the new compact scatter gather mechanism.
>
> An alternative solution would be for the the '.get_pages' and the
> i915_gem_gtt_prepare_object to retry with smaller max gap of the
> amount of PFNs that can be combined together - but with this issue
> discovered during rc7 that might be too risky.
>
> Reported-and-Tested-by: Konrad Rzeszutek Wilk
> CC: Chris Wilson
> CC: Imre Deak
> CC: Daniel Vetter
> CC: David Airlie
> CC:
> Signed-off-by: Konrad Rzeszutek Wilk
Queued for -next (with cc: stable), thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
wrote:
> On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
>> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>> > ("drm/i915: create compact dma scatter lists for gem objects") makes
>> > certain assumptions about the under laying DMA API that are not always
>> > correct.
>> >
>> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
>> > I see:
>> >
>> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err =
>> > -28
>> >
>> > Bit of debugging traced it down to dma_map_sg failing (in
>> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>> >
>> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
>> > the maximum it can handle is a an entry of 512KB of virtual contiguous
>> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>> >
>> > Previous to the above mention git commit the SG entries were of 4KB, and
>> > the code introduced by above git commit squashed the CPU contiguous PFNs
>> > in one big virtual address provided to DMA API.
>> >
>> > This patch is a simple semi-revert - were we emulate the old behavior
>> > if we detect that SWIOTLB is online. If it is not online then we continue
>> > on with the new compact scatter gather mechanism.
>> >
>> > An alternative solution would be for the the '.get_pages' and the
>> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
>> > amount of PFNs that can be combined together - but with this issue
>> > discovered during rc7 that might be too risky.
>> >
>> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>> > CC: Chris Wilson
>> > CC: Imre Deak
>> > CC: Daniel Vetter
>> > CC: David Airlie
>> > CC:
>> > Signed-off-by: Konrad Rzeszutek Wilk
>>
>> Two things:
>
> Hey Daniel,
>
>>
>> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
>> We really rely on the everywhere else true fact that the pages and their
>> dma mapping point at the same backing storage.
>
> It works. As in, it seems to work for just a normal desktop user. I don't
> see much of dma_sync_* sprinkled around the drm/i915 so I would think that
> there are some issues would be hit as well - but at the first glance
> when using it on a laptop it looks OK.
Yeah, we have a pretty serious case of "roll our own coherency stuff".
The biggest reason is that for a long time i915.ko didn't care one bit
about iommus, and the thing we care about (flushing cpu caches for
dma) isn't supported on x86 since x86 every dma is coherent (well, not
quite, but we don't have support for it). I think longer-term it would
make sense to move the clfushing we're doing into the dma layer.
>> - How is this solved elsewhere when constructing sg tables? Or are we
>> really the only guys who try to construct such big sg entries? I
>> expected somewhat that the dma mapping backed would fill in the segment
>> limits accordingly, but I haven't found anything really on a quick
>> search.
>
> The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
> construct the dma mapped pages. That allows it to construct "SWIOTLB-approved"
> pages that won't need to go through dma_map/dma_unmap as they are
> already mapped and ready to go.
>
> Coming back to your question - I think that i915 is the one that I've
> encountered.
That's a bit surprising. With dma_buf graphics people will use sg
tables much more (there's even a nice sg_alloc_table_from_pages helper
to construct them), and those sg tables tend to have large segments. I
guess we need some more generic solution here ...
For now I guess we can live with your CONFIG_SWIOTLB hack.
-Daniel
>>
>>
>> Cheers, Daniel
>>
>> > ---
>> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
>> > 1 file changed, 12 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> > b/drivers/gpu/drm/i915/i915_gem.c
>> > index 970ad17..7045f45 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
>> > drm_i915_gem_object *obj)
>> > gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
>> > gfp &= ~(__GFP_IO | __GFP_WAIT);
>> > }
>> > -
>> > +#ifdef CONFIG_SWIOTLB
>> > + if (swiotlb_nr_tbl()) {
>> > + st->nents++;
>> > + sg_set_page(sg, page, PAGE_SIZE, 0);
>> > + sg = sg_next(sg);
>> > + continue;
>> > + }
>> > +#endif
>> > if (!i || page_to_pfn(page) != last_pfn + 1) {
>> > if (i)
>> > sg = sg_next(sg);
>> > @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
>> > drm_i915_gem_object *obj)
>> >
[PATCH] drm: Added SDP and VSC structures for handling PSR for eDP
From: Shobhit Kumar
SDP header and SDP VSC header as per eDP 1.3 spec, section 3.5,
chapter "PSR Secondary Data Package Support".
v2: Modified and corrected the structures to be more in line for
kernel coding guidelines and rebased the code on Paulo's DP patchset
v3: removing unecessary identation at DP_RECEIVER_CAP_SIZE
v4: moving them to include/drm/drm_dp_helper.h and also already
icluding EDP_PSR_RECEIVER_CAP_SIZE to add everything needed
for PSR at once at drm_dp_helper.h
v5: Fix SDP VSC header and identation by (Paulo Zanoni) and
remove i915 from title (Daniel Vetter)
v6: Fix spec version and move comments from code to commit message
since numbers might change in the future (by Paulo Zanoni).
CC: Paulo Zanoni
Reviewed-by: Paulo Zanoni
Signed-off-by: Sateesh Kavuri
Signed-off-by: Shobhit Kumar
Signed-off-by: Rodrigo Vivi
---
include/drm/drm_dp_helper.h | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index e8e1417..ae8dbfb 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -342,13 +342,42 @@ u8 drm_dp_get_adjust_request_voltage(u8
link_status[DP_LINK_STATUS_SIZE],
u8 drm_dp_get_adjust_request_pre_emphasis(u8 link_status[DP_LINK_STATUS_SIZE],
int lane);
-#define DP_RECEIVER_CAP_SIZE 0xf
+#define DP_RECEIVER_CAP_SIZE 0xf
+#define EDP_PSR_RECEIVER_CAP_SIZE 2
+
void drm_dp_link_train_clock_recovery_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
void drm_dp_link_train_channel_eq_delay(u8 dpcd[DP_RECEIVER_CAP_SIZE]);
u8 drm_dp_link_rate_to_bw_code(int link_rate);
int drm_dp_bw_code_to_link_rate(u8 link_bw);
+struct edp_sdp_header {
+ u8 HB0; /* Secondary Data Packet ID */
+ u8 HB1; /* Secondary Data Packet Type */
+ u8 HB2; /* 7:5 reserved, 4:0 revision number */
+ u8 HB3; /* 7:5 reserved, 4:0 number of valid data bytes */
+} __packed;
+
+#define EDP_SDP_HEADER_REVISION_MASK 0x1F
+#define EDP_SDP_HEADER_VALID_PAYLOAD_BYTES 0x1F
+
+struct edp_vsc_psr {
+ struct edp_sdp_header sdp_header;
+ u8 DB0; /* Stereo Interface */
+ u8 DB1; /* 0 - PSR State; 1 - Update RFB; 2 - CRC Valid */
+ u8 DB2; /* CRC value bits 7:0 of the R or Cr component */
+ u8 DB3; /* CRC value bits 15:8 of the R or Cr component */
+ u8 DB4; /* CRC value bits 7:0 of the G or Y component */
+ u8 DB5; /* CRC value bits 15:8 of the G or Y component */
+ u8 DB6; /* CRC value bits 7:0 of the B or Cb component */
+ u8 DB7; /* CRC value bits 15:8 of the B or Cb component */
+ u8 DB8_31[24]; /* Reserved */
+} __packed;
+
+#define EDP_VSC_PSR_STATE_ACTIVE (1<<0)
+#define EDP_VSC_PSR_UPDATE_RFB (1<<1)
+#define EDP_VSC_PSR_CRC_VALUES_VALID (1<<2)
+
static inline int
drm_dp_max_link_rate(u8 dpcd[DP_RECEIVER_CAP_SIZE])
{
--
1.8.1.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
Dave Airlie wrote:
>On Tue, Jun 25, 2013 at 4:34 AM, Konrad Rzeszutek Wilk
> wrote:
>> On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
>>> On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
>>> wrote:
>>> > On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>>> >> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk
>wrote:
>>> >> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>>> >> > ("drm/i915: create compact dma scatter lists for gem objects")
>makes
>>> >> > certain assumptions about the under laying DMA API that are not
>always
>>> >> > correct.
>>> >> >
>>> >> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the
>bootup
>>> >> > I see:
>>> >> >
>>> >> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>>> >> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on
>[CRTC:3], err = -28
>>> >> >
>>> >> > Bit of debugging traced it down to dma_map_sg failing (in
>>> >> > i915_gem_gtt_prepare_object) as some of the SG entries were
>huge (3MB).
>>> >> >
>>> >> > That unfortunately are sizes that the SWIOTLB is incapable of
>handling -
>>> >> > the maximum it can handle is a an entry of 512KB of virtual
>contiguous
>>> >> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>>> >> >
>>> >> > Previous to the above mention git commit the SG entries were of
>4KB, and
>>> >> > the code introduced by above git commit squashed the CPU
>contiguous PFNs
>>> >> > in one big virtual address provided to DMA API.
>>> >> >
>>> >> > This patch is a simple semi-revert - were we emulate the old
>behavior
>>> >> > if we detect that SWIOTLB is online. If it is not online then
>we continue
>>> >> > on with the new compact scatter gather mechanism.
>>> >> >
>>> >> > An alternative solution would be for the the '.get_pages' and
>the
>>> >> > i915_gem_gtt_prepare_object to retry with smaller max gap of
>the
>>> >> > amount of PFNs that can be combined together - but with this
>issue
>>> >> > discovered during rc7 that might be too risky.
>>> >> >
>>> >> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>
>>> >> > CC: Chris Wilson
>>> >> > CC: Imre Deak
>>> >> > CC: Daniel Vetter
>>> >> > CC: David Airlie
>>> >> > CC:
>>> >> > Signed-off-by: Konrad Rzeszutek Wilk
>>> >>
>>> >> Two things:
>>> >
>>> > Hey Daniel,
>>> >
>>> >>
>>> >> - SWIOTLB usage should seriously blow up all over the place in
>drm/i915.
>>> >> We really rely on the everywhere else true fact that the pages
>and their
>>> >> dma mapping point at the same backing storage.
>>> >
>>> > It works. As in, it seems to work for just a normal desktop user.
>I don't
>>> > see much of dma_sync_* sprinkled around the drm/i915 so I would
>think that
>>> > there are some issues would be hit as well - but at the first
>glance
>>> > when using it on a laptop it looks OK.
>>>
>>> Yeah, we have a pretty serious case of "roll our own coherency
>stuff".
>>> The biggest reason is that for a long time i915.ko didn't care one
>bit
>>> about iommus, and the thing we care about (flushing cpu caches for
>>> dma) isn't supported on x86 since x86 every dma is coherent (well,
>not
>>> quite, but we don't have support for it). I think longer-term it
>would
>>> make sense to move the clfushing we're doing into the dma layer.
>>>
>>> >> - How is this solved elsewhere when constructing sg tables? Or
>are we
>>> >> really the only guys who try to construct such big sg entries?
>I
>>> >> expected somewhat that the dma mapping backed would fill in the
>segment
>>> >> limits accordingly, but I haven't found anything really on a
>quick
>>> >> search.
>>> >
>>> > The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which
>will
>>> > construct the dma mapped pages. That allows it to construct
>"SWIOTLB-approved"
>>> > pages that won't need to go through dma_map/dma_unmap as they are
>>> > already mapped and ready to go.
>>> >
>>> > Coming back to your question - I think that i915 is the one that
>I've
>>> > encountered.
>>>
>>> That's a bit surprising. With dma_buf graphics people will use sg
>>> tables much more (there's even a nice sg_alloc_table_from_pages
>helper
>>> to construct them), and those sg tables tend to have large segments.
>I
>>> guess we need some more generic solution here ...
>>
>> Yes. I don't grok the full picture yet so I am not sure how to help
>with
>> this right now. Is there a roadmap or Wiki on how this was
>envisioned?
>>>
>>> For now I guess we can live with your CONFIG_SWIOTLB hack.
>>> -Daniel
>>
>> OK, I read that as an Ack-ed-by. Should I send the patch to Dave
>Airlie
>> in a GIT PULL or some other way to make it on the v3.10-rc7 train?
>
>I don't like this at all, I'll accept the patch on the condition you
>investigate further :-)
>
>If you are using swiotlb on i915 things should break, I know I've
>investigated problems before where swiotlb was being incorrectly used
>due to page masks or other issues. Shouldn't you be passing through
>using the real iommu?
>
>Dave.
Hey Dave
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> ("drm/i915: create compact dma scatter lists for gem objects") makes
> certain assumptions about the under laying DMA API that are not always
> correct.
>
> On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> I see:
>
> [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
>
> Bit of debugging traced it down to dma_map_sg failing (in
> i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>
> That unfortunately are sizes that the SWIOTLB is incapable of handling -
> the maximum it can handle is a an entry of 512KB of virtual contiguous
> memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>
> Previous to the above mention git commit the SG entries were of 4KB, and
> the code introduced by above git commit squashed the CPU contiguous PFNs
> in one big virtual address provided to DMA API.
>
> This patch is a simple semi-revert - were we emulate the old behavior
> if we detect that SWIOTLB is online. If it is not online then we continue
> on with the new compact scatter gather mechanism.
>
> An alternative solution would be for the the '.get_pages' and the
> i915_gem_gtt_prepare_object to retry with smaller max gap of the
> amount of PFNs that can be combined together - but with this issue
> discovered during rc7 that might be too risky.
>
> Reported-and-Tested-by: Konrad Rzeszutek Wilk
> CC: Chris Wilson
> CC: Imre Deak
> CC: Daniel Vetter
> CC: David Airlie
> CC:
> Signed-off-by: Konrad Rzeszutek Wilk
Two things:
- SWIOTLB usage should seriously blow up all over the place in drm/i915.
We really rely on the everywhere else true fact that the pages and their
dma mapping point at the same backing storage.
- How is this solved elsewhere when constructing sg tables? Or are we
really the only guys who try to construct such big sg entries? I
expected somewhat that the dma mapping backed would fill in the segment
limits accordingly, but I haven't found anything really on a quick
search.
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 15 ---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 970ad17..7045f45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> gfp &= ~(__GFP_IO | __GFP_WAIT);
> }
> -
> +#ifdef CONFIG_SWIOTLB
> + if (swiotlb_nr_tbl()) {
> + st->nents++;
> + sg_set_page(sg, page, PAGE_SIZE, 0);
> + sg = sg_next(sg);
> + continue;
> + }
> +#endif
> if (!i || page_to_pfn(page) != last_pfn + 1) {
> if (i)
> sg = sg_next(sg);
> @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> }
> last_pfn = page_to_pfn(page);
> }
> -
> - sg_mark_end(sg);
> +#ifdef CONFIG_SWIOTLB
> + if (!swiotlb_nr_tbl())
> +#endif
> + sg_mark_end(sg);
> obj->pages = st;
>
> if (i915_gem_object_needs_bit17_swizzle(obj))
> --
> 1.8.1.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH] drm/crtc-helper: don't disable disconnected outputs
On Sun, Jun 16, 2013 at 6:41 AM, Daniel Vetter wrote: > This has originally been added in > > commit a3a0544b2c84e1d7a2022b558ecf66d8c6a8dd93 > Author: Dave Airlie > Date: Mon Aug 31 15:16:30 2009 +1000 > > drm/kms: add explicit encoder disable function and detach harder. > > I think it's the wrong thing to do for a few reasons: > - It's policy in the kernel. Userspace gets hotplug events when an > output disconnects, and it can inquire about all the routing that is > already set up. If userspace wants to disable a disconnected output > it can simply do so itself. > - The reason for adding it given in the commit message was to improve > use of shared encoders. But the disable_unused_functions call only > happens after the modeset has been completed, so it won't help too > much in making the modest succeed. > - We (at least in drm/i915, but iirc all other drivers work the same) > ensure that if the user accidentally yanks the cable and then > replugs, the same configuration will keep on working without any > userspace actions required. For most outputs that's the case by > default, and DP can have the same semantics with a simple re-train > handler fired off from the hpd replug event. This breaks it since if > the user is unlucky and hits the mouse, resulting in a panning of > e.g. the integrated panel that modeset might kill the accidentally > yanked output. Which isn't too great. > > i915 has applied this behaviour change as part of the bit modeset > review, thus far without resulting in an angry crowd with pitchforks: > The reason is that i915 hardware is nearly impossible to ever set something up that could trigger this on. You don't have shared encoders in places that could affect this from what I can see. I know this will break the radeon card I have that I wrote this for originally, where if you alternate hotplugging a tv-out and vga that share the same encoder it won't work. I can probably drag that card out of whatever hole if fell into, but the given justifications have give me no confidence that this patch just isn't reverting the original commit without fixing anything else. I suspect I was testing hotplugging at fbcon not using X also. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 08/23] modetest: Add a command line parameter to set properties
On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> The -w parameter can be used to set a property value from the command
> line, using the target object ID and the property name.
>
> Signed-off-by: Laurent Pinchart
> ---
> tests/modetest/modetest.c | 108
> +-
> 1 file changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 778af62..858d480 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char
> *arg)
> return 0;
> }
>
> +static int parse_property(struct property_arg *p, const char *arg)
> +{
> + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value)
> != 3)
nit: could use stringification to get rid of the magic number 32 here.
I didn't spot any real problems in the series. But I must admit that I
mainly just glanced at most of the changes in since many of the diffs
are a bit hard to read.
I also gave it a quick try using sprites and setting a few modes. And I
found a bug in i915 while doing that, so clearly it has already proved
useful ;)
> + return -1;
> +
> + p->obj_type = 0;
> + p->name[DRM_PROP_NAME_LEN] = '\0';
> +
> + return 0;
> +}
> +
> static void usage(char *name)
> {
> - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
> + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name);
>
> fprintf(stderr, "\n Query options:\n\n");
> fprintf(stderr, "\t-c\tlist connectors\n");
--
Ville Syrj?l?
Intel OTC
[RFC 3/6] drm: add SimpleDRM driver
On 06/24/2013 03:27 PM, David Herrmann wrote: > + sdrm->fb_map = ioremap(sdrm->fb_base, sdrm->fb_size); This should probably be ioremap_wc. Otherwise it will be *really* slow if used in legacy mode and it may cause conflicts with the pgprot_writecombine mode for mmap. (Watching boot messages go by on fbcon on efifb was like using an old 2400 baud modem before I made the corresponding change to efifb.) --Andy
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Tue, Jun 25, 2013 at 9:18 AM, Konrad Rzeszutek Wilk
wrote:
> Dave Airlie wrote:
>
>>On Tue, Jun 25, 2013 at 4:34 AM, Konrad Rzeszutek Wilk
>> wrote:
>>> On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
wrote:
> On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk
>>wrote:
>> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>> > ("drm/i915: create compact dma scatter lists for gem objects")
>>makes
>> > certain assumptions about the under laying DMA API that are not
>>always
>> > correct.
>> >
>> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the
>>bootup
>> > I see:
>> >
>> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on
>>[CRTC:3], err = -28
>> >
>> > Bit of debugging traced it down to dma_map_sg failing (in
>> > i915_gem_gtt_prepare_object) as some of the SG entries were
>>huge (3MB).
>> >
>> > That unfortunately are sizes that the SWIOTLB is incapable of
>>handling -
>> > the maximum it can handle is a an entry of 512KB of virtual
>>contiguous
>> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>> >
>> > Previous to the above mention git commit the SG entries were of
>>4KB, and
>> > the code introduced by above git commit squashed the CPU
>>contiguous PFNs
>> > in one big virtual address provided to DMA API.
>> >
>> > This patch is a simple semi-revert - were we emulate the old
>>behavior
>> > if we detect that SWIOTLB is online. If it is not online then
>>we continue
>> > on with the new compact scatter gather mechanism.
>> >
>> > An alternative solution would be for the the '.get_pages' and
>>the
>> > i915_gem_gtt_prepare_object to retry with smaller max gap of
>>the
>> > amount of PFNs that can be combined together - but with this
>>issue
>> > discovered during rc7 that might be too risky.
>> >
>> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>>
>> > CC: Chris Wilson
>> > CC: Imre Deak
>> > CC: Daniel Vetter
>> > CC: David Airlie
>> > CC:
>> > Signed-off-by: Konrad Rzeszutek Wilk
>>
>> Two things:
>
> Hey Daniel,
>
>>
>> - SWIOTLB usage should seriously blow up all over the place in
>>drm/i915.
>> We really rely on the everywhere else true fact that the pages
>>and their
>> dma mapping point at the same backing storage.
>
> It works. As in, it seems to work for just a normal desktop user.
>>I don't
> see much of dma_sync_* sprinkled around the drm/i915 so I would
>>think that
> there are some issues would be hit as well - but at the first
>>glance
> when using it on a laptop it looks OK.
Yeah, we have a pretty serious case of "roll our own coherency
>>stuff".
The biggest reason is that for a long time i915.ko didn't care one
>>bit
about iommus, and the thing we care about (flushing cpu caches for
dma) isn't supported on x86 since x86 every dma is coherent (well,
>>not
quite, but we don't have support for it). I think longer-term it
>>would
make sense to move the clfushing we're doing into the dma layer.
>> - How is this solved elsewhere when constructing sg tables? Or
>>are we
>> really the only guys who try to construct such big sg entries?
>>I
>> expected somewhat that the dma mapping backed would fill in the
>>segment
>> limits accordingly, but I haven't found anything really on a
>>quick
>> search.
>
> The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which
>>will
> construct the dma mapped pages. That allows it to construct
>>"SWIOTLB-approved"
> pages that won't need to go through dma_map/dma_unmap as they are
> already mapped and ready to go.
>
> Coming back to your question - I think that i915 is the one that
>>I've
> encountered.
That's a bit surprising. With dma_buf graphics people will use sg
tables much more (there's even a nice sg_alloc_table_from_pages
>>helper
to construct them), and those sg tables tend to have large segments.
>>I
guess we need some more generic solution here ...
>>>
>>> Yes. I don't grok the full picture yet so I am not sure how to help
>>with
>>> this right now. Is there a roadmap or Wiki on how this was
>>envisioned?
For now I guess we can live with your CONFIG_SWIOTLB hack.
-Daniel
>>>
>>> OK, I read that as an Ack-ed-by. Should I send the patch to Dave
>>Airlie
>>> in a GIT PULL or some other way to make it on the v3.10-rc7 train?
>>
>>I don't like this at all, I'll accept the patch on the condition you
>>investigate further :-)
>>
>>If you are using swiotlb on i915 things shoul
[PATCH] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
From: YoungJun Cho
Instead of NULL, error value is casted with ERR_PTR() for
drm_prime_pages_to_sg() and IS_ERR_OR_NULL() macro is replaced
with IS_ERR() macro for drm_gem_map_dma_buf().
Signed-off-by: YoungJun Cho
Signed-off-by: Seung-Woo Kim
Signed-off-by: Kyungmin Park
---
drivers/gpu/drm/drm_prime.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index f1699e9..a47eab4 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -74,7 +74,7 @@ static struct sg_table *drm_gem_map_dma_buf(struct
dma_buf_attachment *attach,
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
- if (!IS_ERR_OR_NULL(sgt)) {
+ if (!IS_ERR(sgt)) {
if (!dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir)) {
sg_free_table(sgt);
kfree(sgt);
@@ -417,8 +417,10 @@ struct sg_table *drm_prime_pages_to_sg(struct page
**pages, int nr_pages)
int ret;
sg = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
- if (!sg)
+ if (!sg) {
+ ret = -ENOMEM;
goto out;
+ }
ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
nr_pages << PAGE_SHIFT, GFP_KERNEL);
@@ -428,7 +430,7 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages,
int nr_pages)
return sg;
out:
kfree(sg);
- return NULL;
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(drm_prime_pages_to_sg);
--
1.7.4.1
[PATCH 1/2] power: add new interface to return pm_transition state
On 06/22/2013 03:17 PM, Rafael J. Wysocki wrote: > On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: >> Add a new interface get_pm_transition() to return pm_transition state. >> This interface is intended to be used from dev_pm_ops class and type >> suspend interfaces that call legacy pm ops driver suspend interfaces. >> Legacy suspend pm_ops take pm_message_t as a parameter. >> >> e.g: drm_class_suspend() calls into driver suspend routines >> via drm_dev->driver->suspend(drm_dev, state). >> >> Once drm_class_suspend() is converted to dev_pm_ops, it will no longer >> have access to pm_transition which it has to pass into driver legacy >> suspend calls. get_pm_transition() interface addresses this need. > > That shouldn't be necessary because each transition has its own callback > in strict dev_pm_ops. > > Thanks, > Rafael > Yes that is correct that there is no need pass in or know pm_transition with dev_pm_ops. The issue I am running into is the legacy pm_ops class suspend/resume routines call. In the example, I mentioned in my changelog, drm_class_suspend() calls legacy pm_ops and passes in state. It passes in the state to the driver legacy suspend routine. I have seen code paths in drivers that differentiate between PM_EVENT_FREEZE, PM_EVENT_SUSPEND, PM_EVENT_SLEEP etc. I considered passing PM_EVENT_SUSPEND to drm_dev->driver->suspend(drm_dev, state) from drm_class_suspend() which would eliminate the need for this new interface. However, I am concerned about breaking driver legacy suspend routines that key of off the state to execute different code paths for PM_EVENT_FREEZE vs. PM_EVENT_SUSPEND. Suspend routines get called when state is is PM_EVENT_FREEZE based on my testing. I would rather not add a new interface. Hoping you will have another idea on how to pass in the state to legacy suspend/resume without adding this new interface. My thinking is that this new interface is temporary measure until all of the legacy suspend routines get converted to dev_pm_ops and at the tile legacy interface gets removed, this new interface can go away as well. thanks, -- Shuah Shuah Khan, Linux Kernel Developer - Open Source Group Samsung Research America (Silicon Valley) shuah.kh at samsung.com | (970) 672-0658
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
Dave Airlie wrote:
>On Tue, Jun 25, 2013 at 4:34 AM, Konrad Rzeszutek Wilk
> wrote:
>> On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
>>> On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
>>> wrote:
>>> > On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>>> >> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk
>wrote:
>>> >> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>>> >> > ("drm/i915: create compact dma scatter lists for gem objects")
>makes
>>> >> > certain assumptions about the under laying DMA API that are not
>always
>>> >> > correct.
>>> >> >
>>> >> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the
>bootup
>>> >> > I see:
>>> >> >
>>> >> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>>> >> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on
>[CRTC:3], err = -28
>>> >> >
>>> >> > Bit of debugging traced it down to dma_map_sg failing (in
>>> >> > i915_gem_gtt_prepare_object) as some of the SG entries were
>huge (3MB).
>>> >> >
>>> >> > That unfortunately are sizes that the SWIOTLB is incapable of
>handling -
>>> >> > the maximum it can handle is a an entry of 512KB of virtual
>contiguous
>>> >> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>>> >> >
>>> >> > Previous to the above mention git commit the SG entries were of
>4KB, and
>>> >> > the code introduced by above git commit squashed the CPU
>contiguous PFNs
>>> >> > in one big virtual address provided to DMA API.
>>> >> >
>>> >> > This patch is a simple semi-revert - were we emulate the old
>behavior
>>> >> > if we detect that SWIOTLB is online. If it is not online then
>we continue
>>> >> > on with the new compact scatter gather mechanism.
>>> >> >
>>> >> > An alternative solution would be for the the '.get_pages' and
>the
>>> >> > i915_gem_gtt_prepare_object to retry with smaller max gap of
>the
>>> >> > amount of PFNs that can be combined together - but with this
>issue
>>> >> > discovered during rc7 that might be too risky.
>>> >> >
>>> >> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>
>>> >> > CC: Chris Wilson
>>> >> > CC: Imre Deak
>>> >> > CC: Daniel Vetter
>>> >> > CC: David Airlie
>>> >> > CC:
>>> >> > Signed-off-by: Konrad Rzeszutek Wilk
>>> >>
>>> >> Two things:
>>> >
>>> > Hey Daniel,
>>> >
>>> >>
>>> >> - SWIOTLB usage should seriously blow up all over the place in
>drm/i915.
>>> >> We really rely on the everywhere else true fact that the pages
>and their
>>> >> dma mapping point at the same backing storage.
>>> >
>>> > It works. As in, it seems to work for just a normal desktop user.
>I don't
>>> > see much of dma_sync_* sprinkled around the drm/i915 so I would
>think that
>>> > there are some issues would be hit as well - but at the first
>glance
>>> > when using it on a laptop it looks OK.
>>>
>>> Yeah, we have a pretty serious case of "roll our own coherency
>stuff".
>>> The biggest reason is that for a long time i915.ko didn't care one
>bit
>>> about iommus, and the thing we care about (flushing cpu caches for
>>> dma) isn't supported on x86 since x86 every dma is coherent (well,
>not
>>> quite, but we don't have support for it). I think longer-term it
>would
>>> make sense to move the clfushing we're doing into the dma layer.
>>>
>>> >> - How is this solved elsewhere when constructing sg tables? Or
>are we
>>> >> really the only guys who try to construct such big sg entries?
>I
>>> >> expected somewhat that the dma mapping backed would fill in the
>segment
>>> >> limits accordingly, but I haven't found anything really on a
>quick
>>> >> search.
>>> >
>>> > The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which
>will
>>> > construct the dma mapped pages. That allows it to construct
>"SWIOTLB-approved"
>>> > pages that won't need to go through dma_map/dma_unmap as they are
>>> > already mapped and ready to go.
>>> >
>>> > Coming back to your question - I think that i915 is the one that
>I've
>>> > encountered.
>>>
>>> That's a bit surprising. With dma_buf graphics people will use sg
>>> tables much more (there's even a nice sg_alloc_table_from_pages
>helper
>>> to construct them), and those sg tables tend to have large segments.
>I
>>> guess we need some more generic solution here ...
>>
>> Yes. I don't grok the full picture yet so I am not sure how to help
>with
>> this right now. Is there a roadmap or Wiki on how this was
>envisioned?
>>>
>>> For now I guess we can live with your CONFIG_SWIOTLB hack.
>>> -Daniel
>>
>> OK, I read that as an Ack-ed-by. Should I send the patch to Dave
>Airlie
>> in a GIT PULL or some other way to make it on the v3.10-rc7 train?
>
>I don't like this at all, I'll accept the patch on the condition you
>investigate further :-)
>
>If you are using swiotlb on i915 things should break, I know I've
>investigated problems before where swiotlb was being incorrectly used
>due to page masks or other issues. Shouldn't you be passing through
>using the real iommu?
>
>Dave.
Hey Dave
[PATCH] drm/prime: fix to check return of dma_map_sg in prime helper
From: YoungJun Cho
The dma_map_sg(), in map_dma_buf callback operation of prime helper,
can return 0 when it fails to map, so it needs to release related
resources.
Signed-off-by: YoungJun Cho
Signed-off-by: Seung-Woo Kim
Signed-off-by: Kyungmin Park
---
drivers/gpu/drm/drm_prime.c |9 +++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 5b7b911..f1699e9 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -74,8 +74,13 @@ static struct sg_table *drm_gem_map_dma_buf(struct
dma_buf_attachment *attach,
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
- if (!IS_ERR_OR_NULL(sgt))
- dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+ if (!IS_ERR_OR_NULL(sgt)) {
+ if (!dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir)) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ sgt = ERR_PTR(-ENOMEM);
+ }
+ }
mutex_unlock(&obj->dev->struct_mutex);
return sgt;
--
1.7.4.1
[RFC 6/6] drm: nouveau: kick out firmware drivers during probe
Use the new DRM infrastructure to kick out firmware drivers before probing
the real driver.
Signed-off-by: David Herrmann
---
drivers/gpu/drm/nouveau/nouveau_drm.c | 29 ++---
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 383f4e6..418867f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -233,13 +233,11 @@ nouveau_accel_init(struct nouveau_drm *drm)
nouveau_bo_move_init(drm);
}
-static int nouveau_drm_probe(struct pci_dev *pdev,
-const struct pci_device_id *pent)
+static int nouveau_kick_out_firmware(struct drm_device *dev)
{
- struct nouveau_device *device;
struct apertures_struct *aper;
bool boot = false;
- int ret;
+ struct pci_dev *pdev = dev->pdev;
/* remove conflicting drivers (vesafb, efifb etc) */
aper = alloc_apertures(3);
@@ -265,8 +263,18 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
#ifdef CONFIG_X86
boot = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
#endif
- remove_conflicting_framebuffers(aper, "nouveaufb", boot);
- kfree(aper);
+
+ drm_kick_out_firmware(aper, boot);
+ dev->apertures = aper;
+ dev->apert_boot = boot;
+ return 0;
+}
+
+static int nouveau_drm_probe(struct pci_dev *pdev,
+const struct pci_device_id *pent)
+{
+ struct nouveau_device *device;
+ int ret;
ret = nouveau_device_create(pdev, nouveau_name(pdev), pci_name(pdev),
nouveau_config, nouveau_debug, &device);
@@ -294,9 +302,13 @@ nouveau_drm_load(struct drm_device *dev, unsigned long
flags)
struct nouveau_drm *drm;
int ret;
- ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm);
+ ret = nouveau_kick_out_firmware(dev);
if (ret)
return ret;
+
+ ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm);
+ if (ret)
+ goto fail_apert;
lockdep_set_class(&drm->client.mutex, &drm_client_lock_class_key);
dev->dev_private = drm;
@@ -392,6 +404,9 @@ fail_ttm:
nouveau_vga_fini(drm);
fail_device:
nouveau_cli_destroy(&drm->client);
+fail_apert:
+ kfree(dev->apertures);
+ dev->apertures = NULL;
return ret;
}
--
1.8.3.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 5/6] drm: add helpers to kick out firmware drivers
If we load a real hardware DRM driver, we want all firmware drivers to be
unloaded. Historically, this was done via
remove_conflicting_framebuffers(), but for DRM drivers (like SimpleDRM) we
need a new way.
This patch introduces DRIVER_FIRMWARE and DRM apertures to provide a quite
similar way to kick out firmware DRM drivers. Additionally, unlike the
fbdev equivalent, DRM firmware drivers can now query the system whether a
real hardware driver is already loaded and prevent loading themselves.
Signed-off-by: David Herrmann
---
drivers/gpu/drm/drm_pci.c | 1 +
drivers/gpu/drm/drm_platform.c | 1 +
drivers/gpu/drm/drm_stub.c | 107 +
drivers/gpu/drm/drm_usb.c | 1 +
drivers/gpu/drm/simpledrm/simpledrm.h | 1 +
drivers/gpu/drm/simpledrm/simpledrm_drv.c | 3 +-
drivers/gpu/drm/simpledrm/simpledrm_main.c | 34 +
include/drm/drmP.h | 26 +++
8 files changed, 173 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 14194b6..4dcb2a4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -366,6 +366,7 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct
pci_device_id *ent,
}
list_add_tail(&dev->driver_item, &driver->device_list);
+ list_add_tail(&dev->global_item, &drm_devlist);
DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
driver->name, driver->major, driver->minor, driver->patchlevel,
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index b8a282e..94923c8 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -88,6 +88,7 @@ int drm_get_platform_dev(struct platform_device *platdev,
}
list_add_tail(&dev->driver_item, &driver->device_list);
+ list_add_tail(&dev->global_item, &drm_devlist);
mutex_unlock(&drm_global_mutex);
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 16f3ec5..a433ab0 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,6 +46,9 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */
EXPORT_SYMBOL(drm_timestamp_precision);
+LIST_HEAD(drm_devlist);/* device list; protected by global
lock */
+EXPORT_SYMBOL(drm_devlist);
+
/*
* Default to use monotonic timestamps for wait-for-vblank and page-flip
* complete events.
@@ -484,7 +487,9 @@ void drm_put_dev(struct drm_device *dev)
drm_put_minor(&dev->primary);
+ list_del(&dev->global_item);
list_del(&dev->driver_item);
+ kfree(dev->apertures);
kfree(dev->devname);
kfree(dev);
}
@@ -507,3 +512,105 @@ void drm_unplug_dev(struct drm_device *dev)
mutex_unlock(&drm_global_mutex);
}
EXPORT_SYMBOL(drm_unplug_dev);
+
+void drm_unplug_dev_locked(struct drm_device *dev)
+{
+ /* for a USB device */
+ if (drm_core_check_feature(dev, DRIVER_MODESET))
+ drm_unplug_minor(dev->control);
+ drm_unplug_minor(dev->primary);
+
+ drm_device_set_unplugged(dev);
+
+ /* TODO: schedule drm_put_dev if open_count == 0 */
+}
+EXPORT_SYMBOL(drm_unplug_dev_locked);
+
+#define VGA_FB_PHYS 0xa
+
+static bool apertures_overlap(struct drm_device *dev,
+ struct apertures_struct *ap,
+ bool boot)
+{
+ unsigned int i, j;
+ struct aperture *a, *b;
+
+ if (!dev->apertures)
+ return false;
+
+ for (i = 0; i < dev->apertures->count; ++i) {
+ a = &dev->apertures->ranges[i];
+
+ if (boot && a->base == VGA_FB_PHYS)
+ return true;
+
+ for (j = 0; ap && j < ap->count; ++j) {
+ b = &ap->ranges[j];
+ if (a->base <= b->base && a->base + a->size > b->base)
+ return true;
+ if (b->base <= a->base && b->base + b->size > a->base)
+ return true;
+ }
+ }
+
+ return false;
+}
+
+/**
+ * Kick out firmware
+ *
+ * Virtually unplug any firmware graphics devices which overlap the given
+ * region. This must be called with the global-drm-mutex locked.
+ * This calls the kick_out_firmware() callback on any firmware DRM driver and
+ * after that remove_conflicting_framebuffers() to remove legacy fbdev
+ * framebuffers.
+ */
+void drm_kick_out_firmware(struct apertures_struct *ap, bool boot)
+{
+ struct drm_device *dev;
+
+ list_for_each_entry(dev, &drm_devlist, global_item) {
+ if (!drm_core_check_feature(dev, DRIVER_FIRMWARE))
+ continue;
+ if (apertures_overlap(dev, ap, boot)) {
+ DRM_INFO("kicking out firmware %s\n",
+
[RFC 4/6] drm: simpledrm: add fbdev fallback support
Create a simple fbdev device during SimpleDRM setup so legacy user-space
and fbcon can use it.
fbdev deletion is quite buggy. A unregister_framebuffer() call followed by
a printk() causes NULL-derefs in hide_cursor() and other places in the VT
layer. Hence, we leak the fbdev device currently to make the VT layer
happy. This needs to be fixed soon! Otherwise, we need a "depends !VT"
line for SimpleDRM.
Signed-off-by: David Herrmann
---
drivers/gpu/drm/simpledrm/Kconfig | 11 ++
drivers/gpu/drm/simpledrm/Makefile | 4 +
drivers/gpu/drm/simpledrm/simpledrm.h | 22
drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 180
drivers/gpu/drm/simpledrm/simpledrm_main.c | 2 +
5 files changed, 219 insertions(+)
create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
diff --git a/drivers/gpu/drm/simpledrm/Kconfig
b/drivers/gpu/drm/simpledrm/Kconfig
index 1d4f38e..7936211 100644
--- a/drivers/gpu/drm/simpledrm/Kconfig
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -12,7 +12,18 @@ config DRM_SIMPLEDRM
SimpleDRM supports: "simple-framebuffer" DeviceTree objects, x86 VESA
BIOS Extensions (VBE), EFI framebuffers
+ If fbdev support is enabled, this driver will also provide an fbdev
+ compatibility layer.
+
If unsure, say Y.
To compile this driver as a module, choose M here: the
module will be called simpledrm.
+
+config DRM_SIMPLEDRM_FBDEV
+ bool
+ depends on DRM_SIMPLEDRM && FB
+ default y
+ select FB_CFB_FILLRECT
+ select FB_CFB_COPYAREA
+ select FB_CFB_IMAGEBLIT
diff --git a/drivers/gpu/drm/simpledrm/Makefile
b/drivers/gpu/drm/simpledrm/Makefile
index 2d474a5..e77bd9b 100644
--- a/drivers/gpu/drm/simpledrm/Makefile
+++ b/drivers/gpu/drm/simpledrm/Makefile
@@ -2,4 +2,8 @@ ccflags-y := -Iinclude/drm
simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.o
+ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
+ simpledrm-y += simpledrm_fbdev.o
+endif
+
obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h
b/drivers/gpu/drm/simpledrm/simpledrm.h
index 279d847..cfd99f9 100644
--- a/drivers/gpu/drm/simpledrm/simpledrm.h
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -48,6 +48,9 @@ struct sdrm_device {
struct drm_encoder enc;
struct drm_connector conn;
struct drm_display_mode *mode;
+
+ /* fbdev */
+ struct fb_info *fbdev;
};
int sdrm_drm_load(struct drm_device *ddev, unsigned long flags);
@@ -88,4 +91,23 @@ struct sdrm_framebuffer {
#define to_sdrm_fb(x) container_of(x, struct sdrm_framebuffer, base)
+/* simpledrm fbdev helpers */
+
+#ifdef CONFIG_DRM_SIMPLEDRM_FBDEV
+
+void sdrm_fbdev_init(struct sdrm_device *sdrm);
+void sdrm_fbdev_cleanup(struct sdrm_device *sdrm);
+
+#else /* CONFIG_DRM_SIMPLEDRM_FBDEV */
+
+static inline void sdrm_fbdev_init(struct sdrm_device *sdrm)
+{
+}
+
+static inline void sdrm_fbdev_cleanup(struct sdrm_device *sdrm)
+{
+}
+
+#endif /* CONFIG_DRM_SIMPLEDRM_FBDEV */
+
#endif /* SDRM_DRV_H */
diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
new file mode 100644
index 000..40a2696
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c
@@ -0,0 +1,180 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+/*
+ * fbdev compatibility layer
+ * We provide a basic fbdev device for the same framebuffer that is used for
+ * the pseudo CRTC.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include "simpledrm.h"
+
+struct sdrm_fbdev {
+ u32 palette[256];
+};
+
+static int sdrm_fbdev_setcolreg(unsigned regno, unsigned red, unsigned green,
+ unsigned blue, unsigned transp,
+ struct fb_info *info)
+{
+ /*
+* Set a single color register. The values supplied are
+* already rounded down to the hardware's capabilities
+* (according to the entries in the `var' structure). Return != 0 for
+* invalid regno.
+*/
+
+ if (regno >= info->cmap.len)
+ return 1;
+ if (info->var.bits_per_pixel == 8)
+ return -EINVAL;
+ if (regno >= 16)
+ return 0;
+
+ switch (info->var.bits_per_pixel) {
+ case 16:
+ if (info->var.red.offset == 10) {
+ /* 1:5:5:5 */
+ ((u32*) (info->pseudo_palette))[regno] =
+ ((red & 0xf800) >> 1) |
+ ((green & 0xf800) >> 6) |
+
[RFC 3/6] drm: add SimpleDRM driver
The SimpleDRM driver binds to simple-framebuffer devices and provides a
DRM/KMS API. It provides only a single CRTC+encoder+connector combination
plus one initial mode.
Userspace can create one dumb-buffer and attach it to the CRTC. Only if
the buffer is destroyed, a new buffer can be created. The buffer is
directly mapped into user-space, so we have only resources for a single
buffer. Otherwise, shadow buffers plus damage-request would be needed.
Signed-off-by: David Herrmann
---
MAINTAINERS| 8 +
drivers/gpu/drm/Kconfig| 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/simpledrm/Kconfig | 18 ++
drivers/gpu/drm/simpledrm/Makefile | 5 +
drivers/gpu/drm/simpledrm/simpledrm.h | 91
drivers/gpu/drm/simpledrm/simpledrm_drv.c | 230
drivers/gpu/drm/simpledrm/simpledrm_main.c | 330 +
drivers/gpu/drm/simpledrm/simpledrm_mem.c | 254 ++
9 files changed, 939 insertions(+)
create mode 100644 drivers/gpu/drm/simpledrm/Kconfig
create mode 100644 drivers/gpu/drm/simpledrm/Makefile
create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h
create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c
create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_main.c
create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_mem.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..0f651845 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7305,6 +7305,14 @@ S: Odd Fixes
F: drivers/media/platform/sh_vou.c
F: include/media/sh_vou.h
+SIMPLE DRM DRIVER
+M: David Herrmann
+L: dri-devel@lists.freedesktop.org
+T: git git://people.freedesktop.org/~dvdhrm/linux
+S: Maintained
+F: drivers/gpu/drm/simpledrm
+F: include/linux/platform_data/simpledrm.h
+
SIMPLE FIRMWARE INTERFACE (SFI)
M: Len Brown
L: sfi-de...@simplefirmware.org
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index b16c50e..b3ccef6 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -220,3 +220,5 @@ source "drivers/gpu/drm/omapdrm/Kconfig"
source "drivers/gpu/drm/tilcdc/Kconfig"
source "drivers/gpu/drm/qxl/Kconfig"
+
+source "drivers/gpu/drm/simpledrm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c9f2439..59c424d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -52,4 +52,5 @@ obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/
obj-$(CONFIG_DRM_OMAP) += omapdrm/
obj-$(CONFIG_DRM_TILCDC) += tilcdc/
obj-$(CONFIG_DRM_QXL) += qxl/
+obj-$(CONFIG_DRM_SIMPLEDRM) += simpledrm/
obj-y += i2c/
diff --git a/drivers/gpu/drm/simpledrm/Kconfig
b/drivers/gpu/drm/simpledrm/Kconfig
new file mode 100644
index 000..1d4f38e
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Kconfig
@@ -0,0 +1,18 @@
+config DRM_SIMPLEDRM
+ tristate "Simple firmware framebuffer DRM driver"
+ depends on DRM && !FB_SIMPLE
+ help
+ SimpleDRM can run on all systems with pre-initialized graphics
+ hardware. It uses a framebuffer that was initialized during
+ firmware boot. No page-flipping, modesetting or other advanced
+ features are available. However, other DRM drivers can be loaded
+ later and take over from SimpleDRM if they provide real hardware
+ support.
+
+ SimpleDRM supports: "simple-framebuffer" DeviceTree objects, x86 VESA
+ BIOS Extensions (VBE), EFI framebuffers
+
+ If unsure, say Y.
+
+ To compile this driver as a module, choose M here: the
+ module will be called simpledrm.
diff --git a/drivers/gpu/drm/simpledrm/Makefile
b/drivers/gpu/drm/simpledrm/Makefile
new file mode 100644
index 000..2d474a5
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/Makefile
@@ -0,0 +1,5 @@
+ccflags-y := -Iinclude/drm
+
+simpledrm-y := simpledrm_drv.o simpledrm_main.o simpledrm_mem.o
+
+obj-$(CONFIG_DRM_SIMPLEDRM) := simpledrm.o
diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h
b/drivers/gpu/drm/simpledrm/simpledrm.h
new file mode 100644
index 000..279d847
--- /dev/null
+++ b/drivers/gpu/drm/simpledrm/simpledrm.h
@@ -0,0 +1,91 @@
+/*
+ * SimpleDRM firmware framebuffer driver
+ * Copyright (c) 2012-2013 David Herrmann
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#ifndef SDRM_DRV_H
+#define SDRM_DRV_H
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct sdrm_device;
+struct sdrm_gem_object;
+struct sdrm_framebuffer;
+
+/* simpledrm devices */
+
+struct sdrm_device {
+ struct drm_device *ddev;
+
+ /* framebuffer information */
+ const struct simplefb_format *fb_sformat;
+
[RFC 2/6] x86: provide platform-devices for boot-framebuffers
The current situation regarding boot-framebuffers (VGA, VESA/VBE, EFI) on x86 causes troubles when loading multiple fbdev drivers. The global "struct screen_info" does not provide any state-tracking about which drivers use the FBs. request_mem_region() theoretically works, but unfortunately vesafb/efifb ignore it due to quirks for broken boards. Avoid this by creating a "platform-framebuffer" device with a pointer to the "struct screen_info" as platform-data. Drivers can now create platform-drivers and the driver-core will refuse multiple drivers being active simultaneously. We keep the screen_info available for backwards-compatibility. Drivers can be converted in follow-up patches. Apart from "platform-framebuffer" devices, this also introduces a compatibility option for "simple-framebuffer" drivers which recently got introduced for OF based systems. If CONFIG_X86_SYSFB is selected, we try to match the screen_info against a simple-framebuffer supported format. If we succeed, we create a "simple-framebuffer" device instead of a platform-framebuffer. This allows to reuse the simplefb.c driver across architectures and also to introduce a SimpleDRM driver. There is no need to have vesafb.c, efifb.c, simplefb.c and more just to have architecture specific quirks in their setup-routines. Instead, we now move the architecture specific quirks into x86-setup and provide a generic simple-framebuffer. For backwards-compatibility (if strange formats are used), we still allow vesafb/efifb to be loaded simultaneously and pick up all remaining devices. Signed-off-by: David Herrmann --- arch/x86/Kconfig | 18 ++ arch/x86/kernel/Makefile | 1 + arch/x86/kernel/sysfb.c | 157 +++ 3 files changed, 176 insertions(+) create mode 100644 arch/x86/kernel/sysfb.c diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index fe120da..8eb06b5 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -2255,6 +2255,24 @@ config RAPIDIO source "drivers/rapidio/Kconfig" +config X86_SYSFB + bool "Mark VGA/VBE/EFI FB as generic system framebuffer" + help + Firmwares often provide initial graphics framebuffers so the BIOS, + bootloader or kernel can show basic video-output during boot for + user-guidance and debugging. Historically, x86 used the VESA BIOS + Extensions and EFI-framebuffers for this, which are mostly limited + to x86. However, a generic system-framebuffer initialization emerged + recently on some non-x86 architectures. + This option, if enabled, marks VGA/VBE/EFI framebuffers as generic + framebuffers so the new generic system-framebuffer drivers can be + used on x86. + + This breaks any x86-only driver like efifb, vesafb, uvesafb, which + will not work if this is selected. + + If unsure, say N. + endmenu diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 7bd3bd3..1e1005a 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -100,6 +100,7 @@ obj-$(CONFIG_X86_CHECK_BIOS_CORRUPTION) += check.o obj-$(CONFIG_SWIOTLB) += pci-swiotlb.o obj-$(CONFIG_OF) += devicetree.o obj-$(CONFIG_UPROBES) += uprobes.o +obj-y += sysfb.o obj-$(CONFIG_PERF_EVENTS) += perf_regs.o diff --git a/arch/x86/kernel/sysfb.c b/arch/x86/kernel/sysfb.c new file mode 100644 index 000..8272958 --- /dev/null +++ b/arch/x86/kernel/sysfb.c @@ -0,0 +1,157 @@ +/* + * Generic System Framebuffers on x86 + * Copyright (c) 2012-2013 David Herrmann + */ + +/* + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + */ + +/* + * Simple-Framebuffer support for x86 systems + * Create a platform-device for any available boot framebuffer. The + * simple-framebuffer platform device is already available on DT systems, so + * this module parses the global "screen_info" object and creates a suitable + * platform device compatible with the "simple-framebuffer" DT object. If + * the framebuffer is incompatible, we instead create a "platform-framebuffer" + * device and pass the screen_info as platform_data. This allows legacy drivers + * to pick these devices up without messing with simple-framebuffer drivers. + * + * If CONFIG_X86_SYSFB is not selected, we never register "simple-framebuffer" + * platform devices, but only use "platform-framebuffer" devices for + * backwards compatibility. + * + * TODO: We set the dev_id field of all platform-devices to 0. This allows + * other x86 OF/DT parsers to create such devices, too. However, they must + * start at offset 1 for this to work. + */ + +#include +#include +#include +#include +#include +#include +#include
[RFC 1/6] fbdev: simplefb: add init through platform_data
If we create proper platform-devices in x86 boot-code, we can use simplefb
for VBE or EFI framebuffers, too. However, there is normally no OF support
so we introduce a platform_data object so x86 boot-code can pass the
paramaters via plain old platform-data.
This also removes the OF dependency as it is not needed. The headers
provide proper dummies for the case OF is disabled.
Furthermore, we move the FORMAT-definitions to the common platform header
so initialization code can use it to transform "struct screen_info" to
the right format-name.
Signed-off-by: David Herrmann
---
drivers/video/Kconfig | 5 ++--
drivers/video/simplefb.c | 45 +-
include/linux/platform_data/simplefb.h | 40 ++
3 files changed, 76 insertions(+), 14 deletions(-)
create mode 100644 include/linux/platform_data/simplefb.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..22586ee 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2455,7 +2455,7 @@ config FB_HYPERV
config FB_SIMPLE
bool "Simple framebuffer support"
- depends on (FB = y) && OF
+ depends on (FB = y)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2467,8 +2467,7 @@ config FB_SIMPLE
pre-allocated frame buffer surface.
Configuration re: surface address, size, and format must be provided
- through device tree, or potentially plain old platform data in the
- future.
+ through device tree, or plain old platform data.
source "drivers/video/omap/Kconfig"
source "drivers/video/omap2/Kconfig"
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
index e2e9e3e..35e36c5 100644
--- a/drivers/video/simplefb.c
+++ b/drivers/video/simplefb.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
static struct fb_fix_screeninfo simplefb_fix = {
@@ -73,17 +74,8 @@ static struct fb_ops simplefb_ops = {
.fb_imageblit = cfb_imageblit,
};
-struct simplefb_format {
- const char *name;
- u32 bits_per_pixel;
- struct fb_bitfield red;
- struct fb_bitfield green;
- struct fb_bitfield blue;
- struct fb_bitfield transp;
-};
-
static struct simplefb_format simplefb_formats[] = {
- { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0} },
+ SIMPLEFB_FORMATS
};
struct simplefb_params {
@@ -139,6 +131,32 @@ static int simplefb_parse_dt(struct platform_device *pdev,
return 0;
}
+static int simplefb_parse_pd(struct platform_device *pdev,
+struct simplefb_params *params)
+{
+ struct simplefb_platform_data *pd = pdev->dev.platform_data;
+ int i;
+
+ params->width = pd->width;
+ params->height = pd->height;
+ params->stride = pd->stride;
+
+ for (i = 0; i < ARRAY_SIZE(simplefb_formats); i++) {
+ if (strcmp(pd->format, simplefb_formats[i].name))
+ continue;
+
+ params->format = &simplefb_formats[i];
+ break;
+ }
+
+ if (!params->format) {
+ dev_err(&pdev->dev, "Invalid format value\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int simplefb_probe(struct platform_device *pdev)
{
int ret;
@@ -149,7 +167,12 @@ static int simplefb_probe(struct platform_device *pdev)
if (fb_get_options("simplefb", NULL))
return -ENODEV;
- ret = simplefb_parse_dt(pdev, ¶ms);
+ ret = -ENODEV;
+ if (pdev->dev.platform_data)
+ ret = simplefb_parse_pd(pdev, ¶ms);
+ else if (pdev->dev.of_node)
+ ret = simplefb_parse_dt(pdev, ¶ms);
+
if (ret)
return ret;
diff --git a/include/linux/platform_data/simplefb.h
b/include/linux/platform_data/simplefb.h
new file mode 100644
index 000..a18353d
--- /dev/null
+++ b/include/linux/platform_data/simplefb.h
@@ -0,0 +1,40 @@
+/*
+ * simplefb.h - Simple Framebuffer Device
+ *
+ * Copyright (C) 2013 David Herrmann
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __PLATFORM_DATA_SIMPLEFB_H__
+#define __PLATFORM_DATA_SIMPLEFB_H__
+
+#include
+#include
+#include
+
+#define SIMPLEFB_FORMATS \
+ { "r5g6b5", 16, {11, 5}, {5, 6}, {0, 5}, {0, 0}, DRM_FORMAT_RGB565 }
+
+struct simplefb_format {
+ const char *name;
+ u32 bits_per_pixel;
+ struct fb_bitfield red;
+ struct fb_bitfield green;
+ struct fb_bitfield blue;
+ struct fb_bitfield transp;
+ u32 fourcc;
+};
+
+/* the framebuffer size and location is available as IORESOURCE_MEM */
+struct simplefb_platform_data {
+ u32 width;
[RFC 0/6] SimpleDRM Driver (was: dvbe driver)
Hi This is my second revision of the dvbe driver. I renamed it to SimpleDRM to show the resemblence with the recently introduced simplefb.c fbdev driver. The driver is supposed to be the most basic DRM driver similar to efifb.c, vesafb.c, offb.c, simplefb.c, ... It provides a single virtual CRTC+encoder+connector and allows user-space to create one dumb-buffer at a time and attach it. The setup changed slightly. It no longer uses shadow buffers but instead maps the framebuffer directly into userspace. Furthermore, a new infrastructure is used to unload firmware drivers during real hardware drivers probe cycles. Only nouveau was changed to use it, yet. I still have an odd problem when unloading DRM drivers (not just SimpleDRM) with an fbdev fallback. If I call printk() directly after unregister_framebufer(), I get a NULL-deref somewhere in the VT layer (most times hide_cursor()). I haven't figured out exactly where that happens, but I am also very reluctant to spend more time debugging the VT layer. Anyhow, comments welcome. If someone wants to test it, you probably need to add a line to ./include/linux/platform_data/simplefb.h and add the modeline of your VESA/EFI framebuffer. Cheers David David Herrmann (6): fbdev: simplefb: add init through platform_data x86: provide platform-devices for boot-framebuffers drm: add SimpleDRM driver drm: simpledrm: add fbdev fallback support drm: add helpers to kick out firmware drivers drm: nouveau: kick out firmware drivers during probe MAINTAINERS | 8 + arch/x86/Kconfig| 18 ++ arch/x86/kernel/Makefile| 1 + arch/x86/kernel/sysfb.c | 157 drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/drm_pci.c | 1 + drivers/gpu/drm/drm_platform.c | 1 + drivers/gpu/drm/drm_stub.c | 107 drivers/gpu/drm/drm_usb.c | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 29 ++- drivers/gpu/drm/simpledrm/Kconfig | 29 +++ drivers/gpu/drm/simpledrm/Makefile | 9 + drivers/gpu/drm/simpledrm/simpledrm.h | 114 + drivers/gpu/drm/simpledrm/simpledrm_drv.c | 231 ++ drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 180 ++ drivers/gpu/drm/simpledrm/simpledrm_main.c | 366 drivers/gpu/drm/simpledrm/simpledrm_mem.c | 254 +++ drivers/video/Kconfig | 5 +- drivers/video/simplefb.c| 45 +++- include/drm/drmP.h | 26 ++ include/linux/platform_data/simplefb.h | 40 +++ 22 files changed, 1604 insertions(+), 21 deletions(-) create mode 100644 arch/x86/kernel/sysfb.c create mode 100644 drivers/gpu/drm/simpledrm/Kconfig create mode 100644 drivers/gpu/drm/simpledrm/Makefile create mode 100644 drivers/gpu/drm/simpledrm/simpledrm.h create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_drv.c create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_fbdev.c create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_main.c create mode 100644 drivers/gpu/drm/simpledrm/simpledrm_mem.c create mode 100644 include/linux/platform_data/simplefb.h -- 1.8.3.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-intel-fixes
Hi Dave, One remaining regression fix for i915. I've left it in -fixes for more than a week since it's in tricky code, and it took us a few kernel releases to notice the regression at all. The fence leak is especially annoying on gen2/3 and will kill userspace there quickly. For extra paranoia we've added a WARN in -next to catch this, things seem to be solid now. Cheers, Daniel The following changes since commit 7d132055814ef17a6c7b69f342244c410a5e000f: Linux 3.10-rc6 (2013-06-15 11:51:07 -1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-06-24 for you to fetch changes up to 19b2dbde5732170a03bd82cc8bd442cf88d856f7: drm/i915: Restore fences after resume and GPU resets (2013-06-16 01:10:45 +0200) Chris Wilson (1): drm/i915: Restore fences after resume and GPU resets drivers/gpu/drm/i915/i915_drv.h |2 ++ drivers/gpu/drm/i915/i915_gem.c | 22 +- drivers/gpu/drm/i915/i915_suspend.c |1 + 3 files changed, 8 insertions(+), 17 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
> On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
> wrote:
> > On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
> >> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> >> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> >> > ("drm/i915: create compact dma scatter lists for gem objects") makes
> >> > certain assumptions about the under laying DMA API that are not always
> >> > correct.
> >> >
> >> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> >> > I see:
> >> >
> >> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> >> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err
> >> > = -28
> >> >
> >> > Bit of debugging traced it down to dma_map_sg failing (in
> >> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
> >> >
> >> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
> >> > the maximum it can handle is a an entry of 512KB of virtual contiguous
> >> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
> >> >
> >> > Previous to the above mention git commit the SG entries were of 4KB, and
> >> > the code introduced by above git commit squashed the CPU contiguous PFNs
> >> > in one big virtual address provided to DMA API.
> >> >
> >> > This patch is a simple semi-revert - were we emulate the old behavior
> >> > if we detect that SWIOTLB is online. If it is not online then we continue
> >> > on with the new compact scatter gather mechanism.
> >> >
> >> > An alternative solution would be for the the '.get_pages' and the
> >> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
> >> > amount of PFNs that can be combined together - but with this issue
> >> > discovered during rc7 that might be too risky.
> >> >
> >> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
> >> > CC: Chris Wilson
> >> > CC: Imre Deak
> >> > CC: Daniel Vetter
> >> > CC: David Airlie
> >> > CC:
> >> > Signed-off-by: Konrad Rzeszutek Wilk
> >>
> >> Two things:
> >
> > Hey Daniel,
> >
> >>
> >> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
> >> We really rely on the everywhere else true fact that the pages and their
> >> dma mapping point at the same backing storage.
> >
> > It works. As in, it seems to work for just a normal desktop user. I don't
> > see much of dma_sync_* sprinkled around the drm/i915 so I would think that
> > there are some issues would be hit as well - but at the first glance
> > when using it on a laptop it looks OK.
>
> Yeah, we have a pretty serious case of "roll our own coherency stuff".
> The biggest reason is that for a long time i915.ko didn't care one bit
> about iommus, and the thing we care about (flushing cpu caches for
> dma) isn't supported on x86 since x86 every dma is coherent (well, not
> quite, but we don't have support for it). I think longer-term it would
> make sense to move the clfushing we're doing into the dma layer.
>
> >> - How is this solved elsewhere when constructing sg tables? Or are we
> >> really the only guys who try to construct such big sg entries? I
> >> expected somewhat that the dma mapping backed would fill in the segment
> >> limits accordingly, but I haven't found anything really on a quick
> >> search.
> >
> > The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
> > construct the dma mapped pages. That allows it to construct
> > "SWIOTLB-approved"
> > pages that won't need to go through dma_map/dma_unmap as they are
> > already mapped and ready to go.
> >
> > Coming back to your question - I think that i915 is the one that I've
> > encountered.
>
> That's a bit surprising. With dma_buf graphics people will use sg
> tables much more (there's even a nice sg_alloc_table_from_pages helper
> to construct them), and those sg tables tend to have large segments. I
> guess we need some more generic solution here ...
Yes. I don't grok the full picture yet so I am not sure how to help with
this right now. Is there a roadmap or Wiki on how this was envisioned?
>
> For now I guess we can live with your CONFIG_SWIOTLB hack.
> -Daniel
OK, I read that as an Ack-ed-by. Should I send the patch to Dave Airlie
in a GIT PULL or some other way to make it on the v3.10-rc7 train?
>
>
>
> >>
> >>
> >> Cheers, Daniel
> >>
> >> > ---
> >> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
> >> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> >> > b/drivers/gpu/drm/i915/i915_gem.c
> >> > index 970ad17..7045f45 100644
> >> > --- a/drivers/gpu/drm/i915/i915_gem.c
> >> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> >> > drm_i915_gem_object *obj)
> >> > gfp |= __GFP_NORETRY | __GFP_NOWARN |
> >> > __GFP_NO_KSWAPD;
> >> > gfp &= ~(__GFP_IO | __
[Bug 64695] Enabling both MLAA and MLAA color 2D crashes Gnome Shell on Cayman (6950)
https://bugs.freedesktop.org/show_bug.cgi?id=64695 --- Comment #3 from Brian Paul --- Created attachment 81345 --> https://bugs.freedesktop.org/attachment.cgi?id=81345&action=edit Improved error handling for post-process code I don't know if this patch will help with your issue, but please give it a try. -- 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/20130624/7694949d/attachment.html>
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 Alex Deucher changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |NOTABUG --- Comment #4 from Alex Deucher --- The driver is correct. -- 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/20130624/649367db/attachment.html>
[PATCH 1/2] staging: drm/imx: use generic irqchip
On Fri, 21 Jun 2013 14:52:17 +0200, Philipp Zabel
wrote:
> This depends on the patch "genirq: Generic chip: Add linear irq domain
> support"
> and removes the custom IPU irq_chip and irq_domain_ops. Instead, the generic
> irq chip implementation is reused.
>
> Signed-off-by: Philipp Zabel
Acked-by: Grant Likely
> ---
> drivers/staging/imx-drm/ipu-v3/ipu-common.c | 90
> +
> 1 file changed, 26 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> index d629d6d..c135c66 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-common.c
> @@ -986,53 +986,6 @@ static void ipu_err_irq_handler(unsigned int irq, struct
> irq_desc *desc)
> chained_irq_exit(chip, desc);
> }
>
> -static void ipu_ack_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> -
> - ipu_cm_write(ipu, 1 << (irq % 32), IPU_INT_STAT(irq / 32));
> -}
> -
> -static void ipu_unmask_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&ipu->lock, flags);
> -
> - reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
> - reg |= 1 << (irq % 32);
> - ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
> -
> - spin_unlock_irqrestore(&ipu->lock, flags);
> -}
> -
> -static void ipu_mask_irq(struct irq_data *d)
> -{
> - struct ipu_soc *ipu = irq_data_get_irq_chip_data(d);
> - irq_hw_number_t irq = d->hwirq;
> - unsigned long flags;
> - u32 reg;
> -
> - spin_lock_irqsave(&ipu->lock, flags);
> -
> - reg = ipu_cm_read(ipu, IPU_INT_CTRL(irq / 32));
> - reg &= ~(1 << (irq % 32));
> - ipu_cm_write(ipu, reg, IPU_INT_CTRL(irq / 32));
> -
> - spin_unlock_irqrestore(&ipu->lock, flags);
> -}
> -
> -static struct irq_chip ipu_irq_chip = {
> - .name = "IPU",
> - .irq_ack = ipu_ack_irq,
> - .irq_mask = ipu_mask_irq,
> - .irq_unmask = ipu_unmask_irq,
> -};
> -
> int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
> enum ipu_channel_irq irq_type)
> {
> @@ -1171,32 +1124,39 @@ err_register:
> return ret;
> }
>
> -static int ipu_irq_map(struct irq_domain *h, unsigned int irq,
> -irq_hw_number_t hw)
> -{
> - struct ipu_soc *ipu = h->host_data;
> -
> - irq_set_chip_and_handler(irq, &ipu_irq_chip, handle_level_irq);
> - set_irq_flags(irq, IRQF_VALID);
> - irq_set_chip_data(irq, ipu);
> -
> - return 0;
> -}
> -
> -const struct irq_domain_ops ipu_irq_domain_ops = {
> - .map = ipu_irq_map,
> - .xlate = irq_domain_xlate_onecell,
> -};
>
> static int ipu_irq_init(struct ipu_soc *ipu)
> {
> + struct irq_chip_generic *gc;
> + struct irq_chip_type *ct;
> + int ret, i;
> +
> ipu->domain = irq_domain_add_linear(ipu->dev->of_node, IPU_NUM_IRQS,
> - &ipu_irq_domain_ops, ipu);
> + &irq_generic_chip_ops, ipu);
> if (!ipu->domain) {
> dev_err(ipu->dev, "failed to add irq domain\n");
> return -ENODEV;
> }
>
> + ret = irq_alloc_domain_generic_chips(ipu->domain, 32, 1, "IPU",
> + handle_level_irq, 0, IRQF_VALID,
> 0);
> + if (ret < 0) {
> + dev_err(ipu->dev, "failed to alloc generic irq chips\n");
> + irq_domain_remove(ipu->domain);
> + return ret;
> + }
> +
> + for (i = 0; i < IPU_NUM_IRQS; i += 32) {
> + gc = irq_get_domain_generic_chip(ipu->domain, i);
> + gc->reg_base = ipu->cm_reg;
> + ct = gc->chip_types;
> + ct->chip.irq_ack = irq_gc_ack_set_bit;
> + ct->chip.irq_mask = irq_gc_mask_clr_bit;
> + ct->chip.irq_unmask = irq_gc_mask_set_bit;
> + ct->regs.ack = IPU_INT_STAT(i / 32);
> + ct->regs.mask = IPU_INT_CTRL(i / 32);
> + }
> +
> irq_set_chained_handler(ipu->irq_sync, ipu_irq_handler);
> irq_set_handler_data(ipu->irq_sync, ipu);
> irq_set_chained_handler(ipu->irq_err, ipu_err_irq_handler);
> @@ -1214,6 +1174,8 @@ static void ipu_irq_exit(struct ipu_soc *ipu)
> irq_set_chained_handler(ipu->irq_sync, NULL);
> irq_set_handler_data(ipu->irq_sync, NULL);
>
> + /* TODO: remove irq_domain_generic_chips */
> +
> for (i = 0; i < IPU_NUM_IRQS; i++) {
> irq = irq_linear_revmap(ipu->domain, i);
> if (irq)
> --
> 1.8.3.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
email sent from notm
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 --- Comment #3 from Marek Ol??k --- The "RV530" driver string isn't actually the name of your GPU, it's a category that includes all RV53x GPUs, because the hw programming is the same for all of those from a Mesa driver's point of view. Same for the kernel. -- 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/20130624/866394a4/attachment.html>
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Tue, Jun 25, 2013 at 4:34 AM, Konrad Rzeszutek Wilk
wrote:
> On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
>> On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
>> wrote:
>> > On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>> >> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
>> >> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>> >> > ("drm/i915: create compact dma scatter lists for gem objects") makes
>> >> > certain assumptions about the under laying DMA API that are not always
>> >> > correct.
>> >> >
>> >> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
>> >> > I see:
>> >> >
>> >> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>> >> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err
>> >> > = -28
>> >> >
>> >> > Bit of debugging traced it down to dma_map_sg failing (in
>> >> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>> >> >
>> >> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
>> >> > the maximum it can handle is a an entry of 512KB of virtual contiguous
>> >> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>> >> >
>> >> > Previous to the above mention git commit the SG entries were of 4KB, and
>> >> > the code introduced by above git commit squashed the CPU contiguous PFNs
>> >> > in one big virtual address provided to DMA API.
>> >> >
>> >> > This patch is a simple semi-revert - were we emulate the old behavior
>> >> > if we detect that SWIOTLB is online. If it is not online then we
>> >> > continue
>> >> > on with the new compact scatter gather mechanism.
>> >> >
>> >> > An alternative solution would be for the the '.get_pages' and the
>> >> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
>> >> > amount of PFNs that can be combined together - but with this issue
>> >> > discovered during rc7 that might be too risky.
>> >> >
>> >> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>> >> > CC: Chris Wilson
>> >> > CC: Imre Deak
>> >> > CC: Daniel Vetter
>> >> > CC: David Airlie
>> >> > CC:
>> >> > Signed-off-by: Konrad Rzeszutek Wilk
>> >>
>> >> Two things:
>> >
>> > Hey Daniel,
>> >
>> >>
>> >> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
>> >> We really rely on the everywhere else true fact that the pages and their
>> >> dma mapping point at the same backing storage.
>> >
>> > It works. As in, it seems to work for just a normal desktop user. I don't
>> > see much of dma_sync_* sprinkled around the drm/i915 so I would think that
>> > there are some issues would be hit as well - but at the first glance
>> > when using it on a laptop it looks OK.
>>
>> Yeah, we have a pretty serious case of "roll our own coherency stuff".
>> The biggest reason is that for a long time i915.ko didn't care one bit
>> about iommus, and the thing we care about (flushing cpu caches for
>> dma) isn't supported on x86 since x86 every dma is coherent (well, not
>> quite, but we don't have support for it). I think longer-term it would
>> make sense to move the clfushing we're doing into the dma layer.
>>
>> >> - How is this solved elsewhere when constructing sg tables? Or are we
>> >> really the only guys who try to construct such big sg entries? I
>> >> expected somewhat that the dma mapping backed would fill in the segment
>> >> limits accordingly, but I haven't found anything really on a quick
>> >> search.
>> >
>> > The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
>> > construct the dma mapped pages. That allows it to construct
>> > "SWIOTLB-approved"
>> > pages that won't need to go through dma_map/dma_unmap as they are
>> > already mapped and ready to go.
>> >
>> > Coming back to your question - I think that i915 is the one that I've
>> > encountered.
>>
>> That's a bit surprising. With dma_buf graphics people will use sg
>> tables much more (there's even a nice sg_alloc_table_from_pages helper
>> to construct them), and those sg tables tend to have large segments. I
>> guess we need some more generic solution here ...
>
> Yes. I don't grok the full picture yet so I am not sure how to help with
> this right now. Is there a roadmap or Wiki on how this was envisioned?
>>
>> For now I guess we can live with your CONFIG_SWIOTLB hack.
>> -Daniel
>
> OK, I read that as an Ack-ed-by. Should I send the patch to Dave Airlie
> in a GIT PULL or some other way to make it on the v3.10-rc7 train?
I don't like this at all, I'll accept the patch on the condition you
investigate further :-)
If you are using swiotlb on i915 things should break, I know I've
investigated problems before where swiotlb was being incorrectly used
due to page masks or other issues. Shouldn't you be passing through
using the real iommu?
Dave.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinf
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 Francis Shim changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |--- --- Comment #2 from Francis Shim --- Thank you for the clarification and the link to the "NotebookCheck" site for further information. After doing further research, it was still unclear whether a mismatch still exists or not. We can rule out using RV350; however, now it appears that RV535 might be a better candidate than RV530. According to the ThinkWiki the graphics chipset is identified as M66 and may be identified as M56GL. See: http://www.thinkwiki.org/wiki/ATI_Mobility_FireGL_V5250 According to the Freedesktop Xorg DRI site the M66 graphics set is matched with RV535. See: http://dri.freedesktop.org/wiki/ATIRadeon/ Could someone check this out, please? -- 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/20130624/2547786c/attachment.html>
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> > ("drm/i915: create compact dma scatter lists for gem objects") makes
> > certain assumptions about the under laying DMA API that are not always
> > correct.
> >
> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> > I see:
> >
> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err =
> > -28
> >
> > Bit of debugging traced it down to dma_map_sg failing (in
> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
> >
> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
> > the maximum it can handle is a an entry of 512KB of virtual contiguous
> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
> >
> > Previous to the above mention git commit the SG entries were of 4KB, and
> > the code introduced by above git commit squashed the CPU contiguous PFNs
> > in one big virtual address provided to DMA API.
> >
> > This patch is a simple semi-revert - were we emulate the old behavior
> > if we detect that SWIOTLB is online. If it is not online then we continue
> > on with the new compact scatter gather mechanism.
> >
> > An alternative solution would be for the the '.get_pages' and the
> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
> > amount of PFNs that can be combined together - but with this issue
> > discovered during rc7 that might be too risky.
> >
> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
> > CC: Chris Wilson
> > CC: Imre Deak
> > CC: Daniel Vetter
> > CC: David Airlie
> > CC:
> > Signed-off-by: Konrad Rzeszutek Wilk
>
> Two things:
Hey Daniel,
>
> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
> We really rely on the everywhere else true fact that the pages and their
> dma mapping point at the same backing storage.
It works. As in, it seems to work for just a normal desktop user. I don't
see much of dma_sync_* sprinkled around the drm/i915 so I would think that
there are some issues would be hit as well - but at the first glance
when using it on a laptop it looks OK.
> - How is this solved elsewhere when constructing sg tables? Or are we
> really the only guys who try to construct such big sg entries? I
> expected somewhat that the dma mapping backed would fill in the segment
> limits accordingly, but I haven't found anything really on a quick
> search.
The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
construct the dma mapped pages. That allows it to construct "SWIOTLB-approved"
pages that won't need to go through dma_map/dma_unmap as they are
already mapped and ready to go.
Coming back to your question - I think that i915 is the one that I've
encountered.
>
>
> Cheers, Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 970ad17..7045f45 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> > drm_i915_gem_object *obj)
> > gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> > gfp &= ~(__GFP_IO | __GFP_WAIT);
> > }
> > -
> > +#ifdef CONFIG_SWIOTLB
> > + if (swiotlb_nr_tbl()) {
> > + st->nents++;
> > + sg_set_page(sg, page, PAGE_SIZE, 0);
> > + sg = sg_next(sg);
> > + continue;
> > + }
> > +#endif
> > if (!i || page_to_pfn(page) != last_pfn + 1) {
> > if (i)
> > sg = sg_next(sg);
> > @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
> > drm_i915_gem_object *obj)
> > }
> > last_pfn = page_to_pfn(page);
> > }
> > -
> > - sg_mark_end(sg);
> > +#ifdef CONFIG_SWIOTLB
> > + if (!swiotlb_nr_tbl())
> > +#endif
> > + sg_mark_end(sg);
> > obj->pages = st;
> >
> > if (i915_gem_object_needs_bit17_swizzle(obj))
> > --
> > 1.8.1.4
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 0/4] IPU DMFC bandwidth allocation fix and cleanups
On Fri, Jun 21, 2013 at 10:57:17AM +0200, Philipp Zabel wrote: > Hi, > > the following patches remove some unused variables, replace some > hard-coded channel numbers with existing descriptive defines, fix > the DMFC bandwidth (or rather: FIFO space) allocation, and update > ipu_page_flip() to immediately set crtc->fb. Acked-by: Sascha Hauer Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] power: add new interface to return pm_transition state
On Monday, June 24, 2013 04:20:06 PM Shuah Khan wrote: > On 06/22/2013 03:17 PM, Rafael J. Wysocki wrote: > > On Saturday, June 22, 2013 02:11:14 PM Shuah Khan wrote: > >> Add a new interface get_pm_transition() to return pm_transition state. > >> This interface is intended to be used from dev_pm_ops class and type > >> suspend interfaces that call legacy pm ops driver suspend interfaces. > >> Legacy suspend pm_ops take pm_message_t as a parameter. > >> > >> e.g: drm_class_suspend() calls into driver suspend routines > >> via drm_dev->driver->suspend(drm_dev, state). > >> > >> Once drm_class_suspend() is converted to dev_pm_ops, it will no longer > >> have access to pm_transition which it has to pass into driver legacy > >> suspend calls. get_pm_transition() interface addresses this need. > > > > That shouldn't be necessary because each transition has its own callback > > in strict dev_pm_ops. > > > > Thanks, > > Rafael > > > > Yes that is correct that there is no need pass in or know pm_transition > with dev_pm_ops. The issue I am running into is the legacy pm_ops class > suspend/resume routines call. In the example, I mentioned in my > changelog, drm_class_suspend() calls legacy pm_ops and passes in state. > It passes in the state to the driver legacy suspend routine. I have seen > code paths in drivers that differentiate between PM_EVENT_FREEZE, > PM_EVENT_SUSPEND, PM_EVENT_SLEEP etc. > > I considered passing PM_EVENT_SUSPEND to > drm_dev->driver->suspend(drm_dev, state) from drm_class_suspend() which > would eliminate the need for this new interface. However, I am concerned > about breaking driver legacy suspend routines that key of off the state > to execute different code paths for PM_EVENT_FREEZE vs. > PM_EVENT_SUSPEND. Suspend routines get called when state is is > PM_EVENT_FREEZE based on my testing. > > I would rather not add a new interface. Hoping you will have another > idea on how to pass in the state to legacy suspend/resume without adding > this new interface. My thinking is that this new interface is temporary > measure until all of the legacy suspend routines get converted to > dev_pm_ops and at the tile legacy interface gets removed, this new > interface can go away as well. There are multiple bus types with legacy callbacks. PCI and platform for two examples and USB does something similar. Please have a look at these. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
("drm/i915: create compact dma scatter lists for gem objects") makes
certain assumptions about the under laying DMA API that are not always
correct.
On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
I see:
[drm:intel_pipe_set_base] *ERROR* pin & fence failed
[drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
Bit of debugging traced it down to dma_map_sg failing (in
i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
That unfortunately are sizes that the SWIOTLB is incapable of handling -
the maximum it can handle is a an entry of 512KB of virtual contiguous
memory for its bounce buffer. (See IO_TLB_SEGSIZE).
Previous to the above mention git commit the SG entries were of 4KB, and
the code introduced by above git commit squashed the CPU contiguous PFNs
in one big virtual address provided to DMA API.
This patch is a simple semi-revert - were we emulate the old behavior
if we detect that SWIOTLB is online. If it is not online then we continue
on with the new compact scatter gather mechanism.
An alternative solution would be for the the '.get_pages' and the
i915_gem_gtt_prepare_object to retry with smaller max gap of the
amount of PFNs that can be combined together - but with this issue
discovered during rc7 that might be too risky.
Reported-and-Tested-by: Konrad Rzeszutek Wilk
CC: Chris Wilson
CC: Imre Deak
CC: Daniel Vetter
CC: David Airlie
CC:
Signed-off-by: Konrad Rzeszutek Wilk
---
drivers/gpu/drm/i915/i915_gem.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 970ad17..7045f45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp &= ~(__GFP_IO | __GFP_WAIT);
}
-
+#ifdef CONFIG_SWIOTLB
+ if (swiotlb_nr_tbl()) {
+ st->nents++;
+ sg_set_page(sg, page, PAGE_SIZE, 0);
+ sg = sg_next(sg);
+ continue;
+ }
+#endif
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
@@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
}
last_pfn = page_to_pfn(page);
}
-
- sg_mark_end(sg);
+#ifdef CONFIG_SWIOTLB
+ if (!swiotlb_nr_tbl())
+#endif
+ sg_mark_end(sg);
obj->pages = st;
if (i915_gem_object_needs_bit17_swizzle(obj))
--
1.8.1.4
[PATCH] Bootup regression of v3.10-rc6 + SWIOTLB + Intel 4000.
Hey Dave, Chris, Imre,
Attached is a fix that makes v3.10-rc6 boot on Intel HD 4000 when SWIOTLB
bounce buffer is in usage. The SWIOTLB can only handle up to 512KB swath
of memory to create bounce buffers for and Imre's patch made it possible
to provide more than to the DMA API which caused it to fail with dma_map_sg.
Since this is rc7 time I did the less risky way of fixing it - by just
doing what said code did before 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
("drm/i915: create compact dma scatter lists for gem objects") was
introduced by using a check to see if SWIOTLB is enabled.
It is not the best fix but I figured the less risky.
drivers/gpu/drm/i915/i915_gem.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
I think that a better approach (in v3.11?) would be to do some form of
retry mechanism: (not compile tested, not run at all):
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b9d00dc..0f9079d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1110,8 +1110,12 @@ struct drm_i915_gem_object_ops {
* will therefore most likely be called when the object itself is
* being released or under memory pressure (where we attempt to
* reap pages for the shrinker).
+*
+* max is the maximum size an sg entry can be. Usually it is
+* PAGE_SIZE but if the backend (IOMMU) can deal with larger
+* then a larger value might be used as well.
*/
- int (*get_pages)(struct drm_i915_gem_object *);
+ int (*get_pages)(struct drm_i915_gem_object *, unsigned long max);
void (*put_pages)(struct drm_i915_gem_object *);
};
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7045f45..a29e7db 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1738,7 +1738,7 @@ i915_gem_shrink_all(struct drm_i915_private *dev_priv)
}
static int
-i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
+i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj, unsigned long
max)
{
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
int page_count, i;
@@ -1809,7 +1809,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
continue;
}
#endif
- if (!i || page_to_pfn(page) != last_pfn + 1) {
+ if ((!i || (page_to_pfn(page) != last_pfn + 1)) && (sg->length
< max)) {
if (i)
sg = sg_next(sg);
st->nents++;
@@ -1847,7 +1847,7 @@ err_pages:
* or as the object is itself released.
*/
int
-i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
+i915_gem_object_get_pages(struct drm_i915_gem_object *obj, unsigned int max)
{
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
const struct drm_i915_gem_object_ops *ops = obj->ops;
@@ -1863,7 +1863,7 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
BUG_ON(obj->pages_pin_count);
- ret = ops->get_pages(obj);
+ ret = ops->get_pages(obj, max);
if (ret)
return ret;
@@ -2942,7 +2942,12 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
u32 size, fence_size, fence_alignment, unfenced_alignment;
bool mappable, fenceable;
int ret;
+ static unsigned int max_size = 4 * 1024 * 1024; /* 4MB */
+#ifdef CONFIG_SWIOTLB
+ if (swiotlb_nr_tbl())
+ max_size = PAGE_SIZE;
+#endif
fence_size = i915_gem_get_gtt_size(dev,
obj->base.size,
obj->tiling_mode);
@@ -2972,8 +2977,8 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
DRM_ERROR("Attempting to bind an object larger than the
aperture\n");
return -E2BIG;
}
-
- ret = i915_gem_object_get_pages(obj);
+ retry:
+ ret = i915_gem_object_get_pages(obj, max_size);
if (ret)
return ret;
@@ -3015,6 +3020,10 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
if (ret) {
i915_gem_object_unpin_pages(obj);
drm_mm_put_block(node);
+ if (max_size > PAGE_SIZE) {
+ max_size >> 1;
+ goto retry;
+ }
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index dc53a52..8101387 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -230,7 +230,8 @@ struct dma_buf *i915_gem_prime_export(struct drm_device
*dev,
return dma_buf_export(obj, &i915_dmabuf_ops, obj->base.size, flags);
}
-static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
+
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 08:26:18PM +0200, Daniel Vetter wrote:
> On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
> wrote:
> > On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
> >> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> >> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> >> > ("drm/i915: create compact dma scatter lists for gem objects") makes
> >> > certain assumptions about the under laying DMA API that are not always
> >> > correct.
> >> >
> >> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> >> > I see:
> >> >
> >> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> >> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err
> >> > = -28
> >> >
> >> > Bit of debugging traced it down to dma_map_sg failing (in
> >> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
> >> >
> >> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
> >> > the maximum it can handle is a an entry of 512KB of virtual contiguous
> >> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
> >> >
> >> > Previous to the above mention git commit the SG entries were of 4KB, and
> >> > the code introduced by above git commit squashed the CPU contiguous PFNs
> >> > in one big virtual address provided to DMA API.
> >> >
> >> > This patch is a simple semi-revert - were we emulate the old behavior
> >> > if we detect that SWIOTLB is online. If it is not online then we continue
> >> > on with the new compact scatter gather mechanism.
> >> >
> >> > An alternative solution would be for the the '.get_pages' and the
> >> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
> >> > amount of PFNs that can be combined together - but with this issue
> >> > discovered during rc7 that might be too risky.
> >> >
> >> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
> >> > CC: Chris Wilson
> >> > CC: Imre Deak
> >> > CC: Daniel Vetter
> >> > CC: David Airlie
> >> > CC:
> >> > Signed-off-by: Konrad Rzeszutek Wilk
> >>
> >> Two things:
> >
> > Hey Daniel,
> >
> >>
> >> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
> >> We really rely on the everywhere else true fact that the pages and their
> >> dma mapping point at the same backing storage.
> >
> > It works. As in, it seems to work for just a normal desktop user. I don't
> > see much of dma_sync_* sprinkled around the drm/i915 so I would think that
> > there are some issues would be hit as well - but at the first glance
> > when using it on a laptop it looks OK.
>
> Yeah, we have a pretty serious case of "roll our own coherency stuff".
> The biggest reason is that for a long time i915.ko didn't care one bit
> about iommus, and the thing we care about (flushing cpu caches for
> dma) isn't supported on x86 since x86 every dma is coherent (well, not
> quite, but we don't have support for it). I think longer-term it would
> make sense to move the clfushing we're doing into the dma layer.
>
> >> - How is this solved elsewhere when constructing sg tables? Or are we
> >> really the only guys who try to construct such big sg entries? I
> >> expected somewhat that the dma mapping backed would fill in the segment
> >> limits accordingly, but I haven't found anything really on a quick
> >> search.
> >
> > The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
> > construct the dma mapped pages. That allows it to construct
> > "SWIOTLB-approved"
> > pages that won't need to go through dma_map/dma_unmap as they are
> > already mapped and ready to go.
> >
> > Coming back to your question - I think that i915 is the one that I've
> > encountered.
>
> That's a bit surprising. With dma_buf graphics people will use sg
> tables much more (there's even a nice sg_alloc_table_from_pages helper
> to construct them), and those sg tables tend to have large segments. I
> guess we need some more generic solution here ...
Yes. I don't grok the full picture yet so I am not sure how to help with
this right now. Is there a roadmap or Wiki on how this was envisioned?
>
> For now I guess we can live with your CONFIG_SWIOTLB hack.
> -Daniel
OK, I read that as an Ack-ed-by. Should I send the patch to Dave Airlie
in a GIT PULL or some other way to make it on the v3.10-rc7 train?
>
>
>
> >>
> >>
> >> Cheers, Daniel
> >>
> >> > ---
> >> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
> >> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> >> > b/drivers/gpu/drm/i915/i915_gem.c
> >> > index 970ad17..7045f45 100644
> >> > --- a/drivers/gpu/drm/i915/i915_gem.c
> >> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> >> > drm_i915_gem_object *obj)
> >> > gfp |= __GFP_NORETRY | __GFP_NOWARN |
> >> > __GFP_NO_KSWAPD;
> >> > gfp &= ~(__GFP_IO | __
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> ("drm/i915: create compact dma scatter lists for gem objects") makes
> certain assumptions about the under laying DMA API that are not always
> correct.
>
> On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> I see:
>
> [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
>
> Bit of debugging traced it down to dma_map_sg failing (in
> i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>
> That unfortunately are sizes that the SWIOTLB is incapable of handling -
> the maximum it can handle is a an entry of 512KB of virtual contiguous
> memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>
> Previous to the above mention git commit the SG entries were of 4KB, and
> the code introduced by above git commit squashed the CPU contiguous PFNs
> in one big virtual address provided to DMA API.
>
> This patch is a simple semi-revert - were we emulate the old behavior
> if we detect that SWIOTLB is online. If it is not online then we continue
> on with the new compact scatter gather mechanism.
>
> An alternative solution would be for the the '.get_pages' and the
> i915_gem_gtt_prepare_object to retry with smaller max gap of the
> amount of PFNs that can be combined together - but with this issue
> discovered during rc7 that might be too risky.
>
> Reported-and-Tested-by: Konrad Rzeszutek Wilk
> CC: Chris Wilson
> CC: Imre Deak
> CC: Daniel Vetter
> CC: David Airlie
> CC:
> Signed-off-by: Konrad Rzeszutek Wilk
Queued for -next (with cc: stable), thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 7:32 PM, Konrad Rzeszutek Wilk
wrote:
> On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
>> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
>> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
>> > ("drm/i915: create compact dma scatter lists for gem objects") makes
>> > certain assumptions about the under laying DMA API that are not always
>> > correct.
>> >
>> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
>> > I see:
>> >
>> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
>> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err =
>> > -28
>> >
>> > Bit of debugging traced it down to dma_map_sg failing (in
>> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>> >
>> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
>> > the maximum it can handle is a an entry of 512KB of virtual contiguous
>> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>> >
>> > Previous to the above mention git commit the SG entries were of 4KB, and
>> > the code introduced by above git commit squashed the CPU contiguous PFNs
>> > in one big virtual address provided to DMA API.
>> >
>> > This patch is a simple semi-revert - were we emulate the old behavior
>> > if we detect that SWIOTLB is online. If it is not online then we continue
>> > on with the new compact scatter gather mechanism.
>> >
>> > An alternative solution would be for the the '.get_pages' and the
>> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
>> > amount of PFNs that can be combined together - but with this issue
>> > discovered during rc7 that might be too risky.
>> >
>> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
>> > CC: Chris Wilson
>> > CC: Imre Deak
>> > CC: Daniel Vetter
>> > CC: David Airlie
>> > CC:
>> > Signed-off-by: Konrad Rzeszutek Wilk
>>
>> Two things:
>
> Hey Daniel,
>
>>
>> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
>> We really rely on the everywhere else true fact that the pages and their
>> dma mapping point at the same backing storage.
>
> It works. As in, it seems to work for just a normal desktop user. I don't
> see much of dma_sync_* sprinkled around the drm/i915 so I would think that
> there are some issues would be hit as well - but at the first glance
> when using it on a laptop it looks OK.
Yeah, we have a pretty serious case of "roll our own coherency stuff".
The biggest reason is that for a long time i915.ko didn't care one bit
about iommus, and the thing we care about (flushing cpu caches for
dma) isn't supported on x86 since x86 every dma is coherent (well, not
quite, but we don't have support for it). I think longer-term it would
make sense to move the clfushing we're doing into the dma layer.
>> - How is this solved elsewhere when constructing sg tables? Or are we
>> really the only guys who try to construct such big sg entries? I
>> expected somewhat that the dma mapping backed would fill in the segment
>> limits accordingly, but I haven't found anything really on a quick
>> search.
>
> The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
> construct the dma mapped pages. That allows it to construct "SWIOTLB-approved"
> pages that won't need to go through dma_map/dma_unmap as they are
> already mapped and ready to go.
>
> Coming back to your question - I think that i915 is the one that I've
> encountered.
That's a bit surprising. With dma_buf graphics people will use sg
tables much more (there's even a nice sg_alloc_table_from_pages helper
to construct them), and those sg tables tend to have large segments. I
guess we need some more generic solution here ...
For now I guess we can live with your CONFIG_SWIOTLB hack.
-Daniel
>>
>>
>> Cheers, Daniel
>>
>> > ---
>> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
>> > 1 file changed, 12 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> > b/drivers/gpu/drm/i915/i915_gem.c
>> > index 970ad17..7045f45 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem.c
>> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
>> > drm_i915_gem_object *obj)
>> > gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
>> > gfp &= ~(__GFP_IO | __GFP_WAIT);
>> > }
>> > -
>> > +#ifdef CONFIG_SWIOTLB
>> > + if (swiotlb_nr_tbl()) {
>> > + st->nents++;
>> > + sg_set_page(sg, page, PAGE_SIZE, 0);
>> > + sg = sg_next(sg);
>> > + continue;
>> > + }
>> > +#endif
>> > if (!i || page_to_pfn(page) != last_pfn + 1) {
>> > if (i)
>> > sg = sg_next(sg);
>> > @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
>> > drm_i915_gem_object *obj)
>> >
Regression introduced by 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4 ("drm/i915: create compact dma scatter lists for gem objects") Was:Re: i915 mapping large (3MB) scatter list, hitting limits on certai
On Sat, Jun 22, 2013 at 03:22:59PM +0100, Chris Wilson wrote:
> On Fri, Jun 21, 2013 at 10:03:43PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 21, 2013 at 03:28:28PM -0400, Konrad Rzeszutek Wilk wrote:
> > > Hey,
> >
> > CC-ing Imre,
> >
> > Imre, your patch 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> > ("drm/i915: create compact dma scatter lists for gem objects") is the cause
> > of the regression.
> >
> > If I revert your patch it boots fine without any trouble.
> >
> > I am not entirely sure why that is - as I added some debug code in
> > lib/swiotlb.c to trigger when it can't find 3MB area (which
> > is what I thought initially was the issue) - but none of the debug
> > code seems to be hit.
> >
> > Any thoughts?
>
> You should be hitting drivers/iommu/intel-iommu.c for the dma
> translation. It looks like the contiguous 3MiB segment will be special
> as it is the first sg that __domain_mapping() will attempt to allocate a
> superpage (2MiB) for. What goes wrong at point, I am not sure, but I
> would suggest peppering intel-iommu.c with printk to track down the error.
I figured it out. The issue was that I am backed by the SWIOTLB which
can only allocate up to 128*4K chunks of contingous bounce buffer
(see IO_TLB_SEGSIZE) - which means it can only setup up to 512kB buffers.
While one of the SG entries tries to give it one past that size (3MB).
The change Imre introduced assume that the CPU addresses (virtual) are
the same as the bus addresses. That is correct in most platforms, but some
(for example when booting a Xen PV guest with i915 as PCI passthrough)
the virt_to_phys() values != bus address. Which means that the nice check of:
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
st->nents++;
sg_set_page(sg, page, PAGE_SIZE, 0);
} else {
sg->length += PAGE_SIZE;
}
is too simplistic. What it ought to do is consult the DMA API whether the
next PFN (page_to_pfn(page)) is really contingous in the DMA space. And also
that it does got past the DMA mask for said device.
Unfortunatly such calls do not exist. Those checks are all done when
dma_map_page_* is done (which is where it failed for me).
The best (for an rc7 stage) fix I came up with is to revert just a bit
of the old behavior and still retain the sg compact code behavior.
See following patch:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 970ad17..9edd2eb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1801,7 +1801,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp &= ~(__GFP_IO | __GFP_WAIT);
}
-
+ if (swiotlb_nr_tbl()) {
+ st->nents++;
+ sg_set_page(sg, page, PAGE_SIZE, 0);
+ sg = sg_next(sg);
+ continue;
+ }
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
@@ -1813,7 +1818,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
last_pfn = page_to_pfn(page);
}
- sg_mark_end(sg);
+ if (!swiotlb_nr_tbl())
+ sg_mark_end(sg);
obj->pages = st;
if (i915_gem_object_needs_bit17_swizzle(obj))
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 07:09:12PM +0200, Daniel Vetter wrote:
> On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> > Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> > ("drm/i915: create compact dma scatter lists for gem objects") makes
> > certain assumptions about the under laying DMA API that are not always
> > correct.
> >
> > On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> > I see:
> >
> > [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> > [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err =
> > -28
> >
> > Bit of debugging traced it down to dma_map_sg failing (in
> > i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
> >
> > That unfortunately are sizes that the SWIOTLB is incapable of handling -
> > the maximum it can handle is a an entry of 512KB of virtual contiguous
> > memory for its bounce buffer. (See IO_TLB_SEGSIZE).
> >
> > Previous to the above mention git commit the SG entries were of 4KB, and
> > the code introduced by above git commit squashed the CPU contiguous PFNs
> > in one big virtual address provided to DMA API.
> >
> > This patch is a simple semi-revert - were we emulate the old behavior
> > if we detect that SWIOTLB is online. If it is not online then we continue
> > on with the new compact scatter gather mechanism.
> >
> > An alternative solution would be for the the '.get_pages' and the
> > i915_gem_gtt_prepare_object to retry with smaller max gap of the
> > amount of PFNs that can be combined together - but with this issue
> > discovered during rc7 that might be too risky.
> >
> > Reported-and-Tested-by: Konrad Rzeszutek Wilk
> > CC: Chris Wilson
> > CC: Imre Deak
> > CC: Daniel Vetter
> > CC: David Airlie
> > CC:
> > Signed-off-by: Konrad Rzeszutek Wilk
>
> Two things:
Hey Daniel,
>
> - SWIOTLB usage should seriously blow up all over the place in drm/i915.
> We really rely on the everywhere else true fact that the pages and their
> dma mapping point at the same backing storage.
It works. As in, it seems to work for just a normal desktop user. I don't
see much of dma_sync_* sprinkled around the drm/i915 so I would think that
there are some issues would be hit as well - but at the first glance
when using it on a laptop it looks OK.
> - How is this solved elsewhere when constructing sg tables? Or are we
> really the only guys who try to construct such big sg entries? I
> expected somewhat that the dma mapping backed would fill in the segment
> limits accordingly, but I haven't found anything really on a quick
> search.
The TTM layer (so radeon, nouveau) uses pci_alloc_coherent which will
construct the dma mapped pages. That allows it to construct "SWIOTLB-approved"
pages that won't need to go through dma_map/dma_unmap as they are
already mapped and ready to go.
Coming back to your question - I think that i915 is the one that I've
encountered.
>
>
> Cheers, Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 970ad17..7045f45 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> > drm_i915_gem_object *obj)
> > gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> > gfp &= ~(__GFP_IO | __GFP_WAIT);
> > }
> > -
> > +#ifdef CONFIG_SWIOTLB
> > + if (swiotlb_nr_tbl()) {
> > + st->nents++;
> > + sg_set_page(sg, page, PAGE_SIZE, 0);
> > + sg = sg_next(sg);
> > + continue;
> > + }
> > +#endif
> > if (!i || page_to_pfn(page) != last_pfn + 1) {
> > if (i)
> > sg = sg_next(sg);
> > @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
> > drm_i915_gem_object *obj)
> > }
> > last_pfn = page_to_pfn(page);
> > }
> > -
> > - sg_mark_end(sg);
> > +#ifdef CONFIG_SWIOTLB
> > + if (!swiotlb_nr_tbl())
> > +#endif
> > + sg_mark_end(sg);
> > obj->pages = st;
> >
> > if (i915_gem_object_needs_bit17_swizzle(obj))
> > --
> > 1.8.1.4
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/7] mutex: add support for wound/wait style locks
Op 20-06-13 14:23, Ingo Molnar schreef: > * Maarten Lankhorst wrote: > >> Well they've helped me with some of the changes and contributed some >> code and/or fixes, but if acked-by is preferred I'll use that.. > Such contributions can be credited in the changelog, and/or copyright > notices, and/or the code itself. The signoff chain on the other hand is > strictly defined as a 'route the patch took', with a single point of > origin, the main author. See Documentation/SubmittingPatches, pt 12. > > [ A signoff chain _can_ signal multi-authored code where the code got > written by someone and then further fixed/developed by someone else - > who adds a SOB to the end - but in that case I expect to get the patch > from the last person in the signoff chain. ] > > Thanks, > > Ingo Is this better? I added some more to the changelog entry, clarified ttm and fixed the sob's. 8< Wound/wait mutexes are used when other multiple lock acquisitions of a similar type can be done in an arbitrary order. The deadlock handling used here is called wait/wound in the RDBMS literature: The older tasks waits until it can acquire the contended lock. The younger tasks needs to back off and drop all the locks it is currently holding, i.e. the younger task is wounded. For full documentation please read Documentation/ww-mutex-design.txt. Changes since RFC patch v1: - Updated to use atomic_long instead of atomic, since the reservation_id was a long. - added mutex_reserve_lock_slow and mutex_reserve_lock_intr_slow - removed mutex_locked_set_reservation_id (or w/e it was called) Changes since RFC patch v2: - remove use of __mutex_lock_retval_arg, add warnings when using wrong combination of mutex_(,reserve_)lock/unlock. Changes since v1: - Add __always_inline to __mutex_lock_common, otherwise reservation paths can be triggered from normal locks, because __builtin_constant_p might evaluate to false for the constant 0 in that case. Tests for this have been added in the next patch. - Updated documentation slightly. Changes since v2: - Renamed everything to ww_mutex. (mlankhorst) - Added ww_acquire_ctx and ww_class. (mlankhorst) - Added a lot of checks for wrong api usage. (mlankhorst) - Documentation updates. (danvet) Changes since v3: - Small documentation fixes (robclark) - Memory barrier fix (danvet) Changes since v4: - Remove ww_mutex_unlock_single and ww_mutex_lock_single. - Rename ww_mutex_trylock_single to ww_mutex_trylock. - Remove separate implementations of ww_mutex_lock_slow*, normal functions can be used. Inline versions still exist for extra debugging. - Cleanup unneeded memory barriers, add comment to the remaining smp_mb(). Changes since v5: - Clarify TTM -> TTM graphics subsystem References: https://lwn.net/Articles/548909/ Acked-by: Daniel Vetter Acked-by: Rob Clark Signed-off-by: Maarten Lankhorst --- Documentation/ww-mutex-design.txt | 344 include/linux/mutex-debug.h | 1 + include/linux/mutex.h | 355 +- kernel/mutex.c| 318 -- lib/debug_locks.c | 2 + 5 files changed, 1003 insertions(+), 17 deletions(-) create mode 100644 Documentation/ww-mutex-design.txt diff --git a/Documentation/ww-mutex-design.txt b/Documentation/ww-mutex-design.txt new file mode 100644 index 000..8a112dc --- /dev/null +++ b/Documentation/ww-mutex-design.txt @@ -0,0 +1,344 @@ +Wait/Wound Deadlock-Proof Mutex Design +== + +Please read mutex-design.txt first, as it applies to wait/wound mutexes too. + +Motivation for WW-Mutexes +- + +GPU's do operations that commonly involve many buffers. Those buffers +can be shared across contexts/processes, exist in different memory +domains (for example VRAM vs system memory), and so on. And with +PRIME / dmabuf, they can even be shared across devices. So there are +a handful of situations where the driver needs to wait for buffers to +become ready. If you think about this in terms of waiting on a buffer +mutex for it to become available, this presents a problem because +there is no way to guarantee that buffers appear in a execbuf/batch in +the same order in all contexts. That is directly under control of +userspace, and a result of the sequence of GL calls that an application +makes. Which results in the potential for deadlock. The problem gets +more complex when you consider that the kernel may need to migrate the +buffer(s) into VRAM before the GPU operates on the buffer(s), which +may in turn require evicting some other buffers (and you don't want to +evict other buffers which are already queued up to the GPU), but for a +simplified understanding of the problem you can ignore this. + +The algorithm that the TTM graphics subsystem came up with for dealing with +this problem is quite simple. For each group
Re: [PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
On Mon, Jun 24, 2013 at 11:47:48AM -0400, Konrad Rzeszutek Wilk wrote:
> Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> ("drm/i915: create compact dma scatter lists for gem objects") makes
> certain assumptions about the under laying DMA API that are not always
> correct.
>
> On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
> I see:
>
> [drm:intel_pipe_set_base] *ERROR* pin & fence failed
> [drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
>
> Bit of debugging traced it down to dma_map_sg failing (in
> i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
>
> That unfortunately are sizes that the SWIOTLB is incapable of handling -
> the maximum it can handle is a an entry of 512KB of virtual contiguous
> memory for its bounce buffer. (See IO_TLB_SEGSIZE).
>
> Previous to the above mention git commit the SG entries were of 4KB, and
> the code introduced by above git commit squashed the CPU contiguous PFNs
> in one big virtual address provided to DMA API.
>
> This patch is a simple semi-revert - were we emulate the old behavior
> if we detect that SWIOTLB is online. If it is not online then we continue
> on with the new compact scatter gather mechanism.
>
> An alternative solution would be for the the '.get_pages' and the
> i915_gem_gtt_prepare_object to retry with smaller max gap of the
> amount of PFNs that can be combined together - but with this issue
> discovered during rc7 that might be too risky.
>
> Reported-and-Tested-by: Konrad Rzeszutek Wilk
> CC: Chris Wilson
> CC: Imre Deak
> CC: Daniel Vetter
> CC: David Airlie
> CC:
> Signed-off-by: Konrad Rzeszutek Wilk
Two things:
- SWIOTLB usage should seriously blow up all over the place in drm/i915.
We really rely on the everywhere else true fact that the pages and their
dma mapping point at the same backing storage.
- How is this solved elsewhere when constructing sg tables? Or are we
really the only guys who try to construct such big sg entries? I
expected somewhat that the dma mapping backed would fill in the segment
limits accordingly, but I haven't found anything really on a quick
search.
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 15 ---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 970ad17..7045f45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
> gfp &= ~(__GFP_IO | __GFP_WAIT);
> }
> -
> +#ifdef CONFIG_SWIOTLB
> + if (swiotlb_nr_tbl()) {
> + st->nents++;
> + sg_set_page(sg, page, PAGE_SIZE, 0);
> + sg = sg_next(sg);
> + continue;
> + }
> +#endif
> if (!i || page_to_pfn(page) != last_pfn + 1) {
> if (i)
> sg = sg_next(sg);
> @@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct
> drm_i915_gem_object *obj)
> }
> last_pfn = page_to_pfn(page);
> }
> -
> - sg_mark_end(sg);
> +#ifdef CONFIG_SWIOTLB
> + if (!swiotlb_nr_tbl())
> +#endif
> + sg_mark_end(sg);
> obj->pages = st;
>
> if (i915_gem_object_needs_bit17_swizzle(obj))
> --
> 1.8.1.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/5] clk/exynos5420: add clocks for hdmi subsystem
Hi Mr. Kim, v3.11-next/soc-exynos5420 seems correct branch for it, not v3.11-next/samsung-clk. In this case rebasing is not required. It cleanly applies there. regards, Rahul Sharma. On Mon, Jun 24, 2013 at 9:33 AM, Rahul Sharma wrote: > Thanks Mike. > > I will rebase these patches to v3.11-next/samsung-clk branch and post v2. > > regards, > Rahul Sharma. > > On Fri, Jun 21, 2013 at 10:13 PM, Mike Turquette > wrote: >> Quoting Rahul Sharma (2013-06-20 21:47:35) >>> On Fri, Jun 21, 2013 at 10:02 AM, Mike Turquette >>> wrote: >>> > Quoting Kukjin Kim (2013-06-19 07:01:17) >>> >> On 06/19/13 13:04, Rahul Sharma wrote: >>> >> > + mike >>> >> > >>> >> Mike, I'm waiting for your reply on this. If you're OK, let me take this >>> >> series on top of exynos5420 stuff in samsung tree. >>> >> >>> >> Of course, if any concerns, please let me know. >>> > >>> > I never got these patches. I'm not subscribed to devicetree-devel or >>> > linux-samsung so I only got two replies to patch #0, but none of the >>> > code. Can you or Rajul resend? >>> > >>> >>> Sure mike. >> >> Acked-by: Mike Turquette >> >> Regards, >> Mike >> >>> >>> > Thanks, >>> > Mike >>> > >>> >> >>> >> Thanks, >>> >> - Kukjin >>> >> >>> >> > On Tue, Jun 18, 2013 at 8:03 PM, Rahul Sharma>> >> > samsung.com> wrote: >>> >> >> Add clock changes for hdmi subsystem for exynos5250 SoC. These >>> >> >> include addition of new clocks like mout_hdmi and smmu_tv, associating >>> >> >> ID to clk_hdmiphy and some essential corrections. >>> >> >> >>> >> >> This set is based on kukjin's for-next branch at >>> >> >> http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git. >>> >> >> >>> >> >> This set dependents on the following patches which add support for >>> >> >> Exynos5420 >>> >> >> http://www.mail-archive.com/linux-samsung-soc at >>> >> >> vger.kernel.org/msg19264.html >>> >> >> >>> >> >> Rahul Sharma (5): >>> >> >>clk/exynos5420: add sclk_hdmiphy to the list of special clocks >>> >> >>clk/exynos5420: add gate clock for tv sysmmu >>> >> >>clk/exynos5420: fix the order of parents of hdmi mux >>> >> >>clk/exynos5420: add hdmi mux to change parents in hdmi driver >>> >> >>clk/exynos5420: assign sclk_pixel id to pixel clock divider >>> >> >> >>> >> >> .../devicetree/bindings/clock/exynos5420-clock.txt |7 +++ >>> >> >> drivers/clk/samsung/clk-exynos5420.c | 18 >>> >> >> +++--- >>> >> >> 2 files changed, 18 insertions(+), 7 deletions(-) >>> >> >> >>> >> >> -- >>> >> >> 1.7.10.4
[PATCH v2] drm: Improve manual IRQ installation documentation
On Sat, Jun 22, 2013 at 02:10:59PM +0200, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart Hm, a bit a thin commit message, but changes look good. So with the commit message patched up this is Reviewed-by: Daniel Vetter > --- > Documentation/DocBook/drm.tmpl | 118 > - > 1 file changed, 70 insertions(+), 48 deletions(-) > > Changes since v1: > > - Document manual IRQ registration > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 91ee107..a608094 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -186,11 +186,12 @@ > > DRIVER_HAVE_IRQDRIVER_IRQ_SHARED > > - DRIVER_HAVE_IRQ indicates whether the driver has an IRQ > handler. The > - DRM core will automatically register an interrupt handler when > the > - flag is set. DRIVER_IRQ_SHARED indicates whether the device > & > - handler support shared IRQs (note that this is required of PCI > - drivers). > + DRIVER_HAVE_IRQ indicates whether the driver has an IRQ handler > + managed by the DRM Core. The core will support simple IRQ > handler > + installation when the flag is set. The installation process is > + described in . > + DRIVER_IRQ_SHARED indicates whether the device & > handler > + support shared IRQs (note that this is required of PCI > drivers). > > > > @@ -344,50 +345,71 @@ char *date; >The DRM core tries to facilitate IRQ handler registration and >unregistration by providing drm_irq_install > and >drm_irq_uninstall functions. Those functions > only > - support a single interrupt per device. > - > - > - > - Both functions get the device IRQ by calling > - drm_dev_to_irq. This inline function will > call a > - bus-specific operation to retrieve the IRQ number. For platform > devices, > - platform_get_irq(..., 0) is used to retrieve > the > - IRQ number. > - > - > - drm_irq_install starts by calling the > - irq_preinstall driver operation. The > operation > - is optional and must make sure that the interrupt will not get > fired by > - clearing all pending interrupt flags or disabling the interrupt. > - > - > - The IRQ will then be requested by a call to > - request_irq. If the DRIVER_IRQ_SHARED driver > - feature flag is set, a shared (IRQF_SHARED) IRQ handler will be > - requested. > - > - > - The IRQ handler function must be provided as the mandatory > irq_handler > - driver operation. It will get passed directly to > - request_irq and thus has the same prototype > as all > - IRQ handlers. It will get called with a pointer to the DRM device > as the > - second argument. > - > - > - Finally the function calls the optional > - irq_postinstall driver operation. The > operation > - usually enables interrupts (excluding the vblank interrupt, which > is > - enabled separately), but drivers may choose to enable/disable > interrupts > - at a different time. > - > - > - drm_irq_uninstall is similarly used to > uninstall an > - IRQ handler. It starts by waking up all processes waiting on a > vblank > - interrupt to make sure they don't hang, and then calls the optional > - irq_uninstall driver operation. The > operation > - must disable all hardware interrupts. Finally the function frees > the IRQ > - by calling free_irq. > + support a single interrupt per device, devices that use more than > one > + IRQs need to be handled manually. > > + > + Managed IRQ Registration > + > +Both the drm_irq_install and > + drm_irq_uninstall functions get the device IRQ > by > + calling drm_dev_to_irq. This inline function > will > + call a bus-specific operation to retrieve the IRQ number. For > platform > + devices, platform_get_irq(..., 0) is used to > + retrieve the IRQ number. > + > + > +drm_irq_install starts by calling the > +irq_preinstall driver operation. The > operation > +is optional and must make sure that the interrupt will not get > fired by > +clearing all pending interrupt flags or disabling the interrupt. > + > + > +The IRQ will then be requested by a call to > +request_irq. If the DRIVER_IRQ_SHARED driver > +feature flag is set, a shared (IRQF_SHARED) IRQ han
[PATCH 0/5] clk/exynos5420: add clocks for hdmi subsystem
Thanks Mike. I will rebase these patches to v3.11-next/samsung-clk branch and post v2. regards, Rahul Sharma. On Fri, Jun 21, 2013 at 10:13 PM, Mike Turquette wrote: > Quoting Rahul Sharma (2013-06-20 21:47:35) >> On Fri, Jun 21, 2013 at 10:02 AM, Mike Turquette >> wrote: >> > Quoting Kukjin Kim (2013-06-19 07:01:17) >> >> On 06/19/13 13:04, Rahul Sharma wrote: >> >> > + mike >> >> > >> >> Mike, I'm waiting for your reply on this. If you're OK, let me take this >> >> series on top of exynos5420 stuff in samsung tree. >> >> >> >> Of course, if any concerns, please let me know. >> > >> > I never got these patches. I'm not subscribed to devicetree-devel or >> > linux-samsung so I only got two replies to patch #0, but none of the >> > code. Can you or Rajul resend? >> > >> >> Sure mike. > > Acked-by: Mike Turquette > > Regards, > Mike > >> >> > Thanks, >> > Mike >> > >> >> >> >> Thanks, >> >> - Kukjin >> >> >> >> > On Tue, Jun 18, 2013 at 8:03 PM, Rahul Sharma> >> > samsung.com> wrote: >> >> >> Add clock changes for hdmi subsystem for exynos5250 SoC. These >> >> >> include addition of new clocks like mout_hdmi and smmu_tv, associating >> >> >> ID to clk_hdmiphy and some essential corrections. >> >> >> >> >> >> This set is based on kukjin's for-next branch at >> >> >> http://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git. >> >> >> >> >> >> This set dependents on the following patches which add support for >> >> >> Exynos5420 >> >> >> http://www.mail-archive.com/linux-samsung-soc at >> >> >> vger.kernel.org/msg19264.html >> >> >> >> >> >> Rahul Sharma (5): >> >> >>clk/exynos5420: add sclk_hdmiphy to the list of special clocks >> >> >>clk/exynos5420: add gate clock for tv sysmmu >> >> >>clk/exynos5420: fix the order of parents of hdmi mux >> >> >>clk/exynos5420: add hdmi mux to change parents in hdmi driver >> >> >>clk/exynos5420: assign sclk_pixel id to pixel clock divider >> >> >> >> >> >> .../devicetree/bindings/clock/exynos5420-clock.txt |7 +++ >> >> >> drivers/clk/samsung/clk-exynos5420.c | 18 >> >> >> +++--- >> >> >> 2 files changed, 18 insertions(+), 7 deletions(-) >> >> >> >> >> >> -- >> >> >> 1.7.10.4
[Intel-gfx] [PATCH] drm/i915: move i915_trace_irq_get() out of the tracing macro
On Fri, Jun 21, 2013 at 02:51:07PM +0200, Sebastian Andrzej Siewior wrote: > On 06/21/2013 01:08 PM, Chris Wilson wrote: > > On Fri, Jun 21, 2013 at 12:15:53PM +0200, Sebastian Andrzej Siewior wrote: > >> There is a report on RT about "BUG: scheduling while atomic" because the > >> sleeping lock is taken in tracing context. This patch simply moves > >> locking operation out of the tracing macro. > > > > No. This enables the IRQ, as well as making a number of > > very expensively serialised read, unconditionally. > > Ach in case CONFIG_TRACING is enabled but the tracepoint itself is > disabled. In that case it is probably best to drop this tracepoint from > -RT. Iirc when we've last discussed this with Thomas Gleixner he mentioned that there's a setup/teardown hook when enabling/disabling a tracepoint. We could use that to enable/disable interrupts. Needs a notch of logic rework though since the auto-irq disable code needs adjusting ... And on a 2nd look that trace_irq_seqno logic looks a bit broken anyway. It's somewhere on my todo list, but patches very much welcome. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 1/2] drm: add hotspot support for cursors.
On Thu, Jun 20, 2013 at 11:48:52AM +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> So it looks like for virtual hw cursors on QXL we need to inform
> the "hw" device what the cursor hotspot parameters are. This
> makes sense if you think the host has to draw the cursor and interpret
> clicks from it. However the current modesetting interface doesn't support
> passing the hotspot information from userspace.
>
> This implements a new cursor ioctl, that takes the hotspot info as well,
> userspace can try calling the new interface and if it -ENOSYS, can just
> fallback to the old non-hotspot interface.
This needs to be updated since your patch implements the fallback to the
old interface transparently already. Otherwise looks sane.
Reviewed-by: Daniel Vetter
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_crtc.c | 35 +--
> drivers/gpu/drm/drm_drv.c | 1 +
> include/drm/drm_crtc.h | 5 +
> include/uapi/drm/drm.h | 1 +
> include/uapi/drm/drm_mode.h | 13 +
> 5 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e7e9242..cc9eada 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2099,10 +2099,10 @@ out:
> return ret;
> }
>
> -int drm_mode_cursor_ioctl(struct drm_device *dev,
> - void *data, struct drm_file *file_priv)
> +static int drm_mode_cursor_common(struct drm_device *dev,
> + struct drm_mode_cursor2 *req,
> + struct drm_file *file_priv)
> {
> - struct drm_mode_cursor *req = data;
> struct drm_mode_object *obj;
> struct drm_crtc *crtc;
> int ret = 0;
> @@ -2122,13 +2122,17 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
>
> mutex_lock(&crtc->mutex);
> if (req->flags & DRM_MODE_CURSOR_BO) {
> - if (!crtc->funcs->cursor_set) {
> + if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
> ret = -ENXIO;
> goto out;
> }
> /* Turns off the cursor if handle is 0 */
> - ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
> - req->width, req->height);
> + if (crtc->funcs->cursor_set2)
> + ret = crtc->funcs->cursor_set2(crtc, file_priv,
> req->handle,
> + req->width, req->height,
> req->hot_x, req->hot_y);
> + else
> + ret = crtc->funcs->cursor_set(crtc, file_priv,
> req->handle,
> + req->width, req->height);
> }
>
> if (req->flags & DRM_MODE_CURSOR_MOVE) {
> @@ -2143,6 +2147,25 @@ out:
> mutex_unlock(&crtc->mutex);
>
> return ret;
> +
> +}
> +int drm_mode_cursor_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_cursor *req = data;
> + struct drm_mode_cursor2 new_req;
> +
> + memcpy(&new_req, req, sizeof(struct drm_mode_cursor));
> + new_req.hot_x = new_req.hot_y = 0;
> +
> + return drm_mode_cursor_common(dev, &new_req, file_priv);
> +}
> +
> +int drm_mode_cursor2_ioctl(struct drm_device *dev,
> +void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_cursor2 *req = data;
> + return drm_mode_cursor_common(dev, req, file_priv);
> }
>
> /* Original addfb only supported RGB formats, so figure out which one */
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9cc247f..99fcd7c 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -166,6 +166,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl,
> DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES,
> drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY,
> drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl,
> DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index adb3f9b..093c030 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -339,6 +339,9 @@ struct drm_crtc_funcs {
> /* cursor controls */
> int (*cursor_set)(struct drm_crtc *crtc, struct drm_file *file_priv,
> uint32_t handle, uint32_t width, uint32_t height);
> + int (*cursor_set2)(struct drm_crtc *crtc, struct drm_file *file_priv,
> +uint32_t handle, uint32_t width, ui
[PATCH] drm: Improve manual IRQ installation documentation
On Sat, Jun 22, 2013 at 12:56:46PM +0200, Thierry Reding wrote: > On Thu, Jun 20, 2013 at 04:55:03PM +0200, Daniel Vetter wrote: > > On Thu, Jun 20, 2013 at 01:00:36PM +0200, Thierry Reding wrote: > > > On Thu, Jun 20, 2013 at 12:46:21PM +0200, Laurent Pinchart wrote: > > > > Hi Thierry, > > > > > > > > On Thursday 20 June 2013 12:40:26 Thierry Reding wrote: > > > > > On Thu, Jun 20, 2013 at 12:17:25PM +0200, Laurent Pinchart wrote: > > > > > > On Thursday 20 June 2013 12:10:47 Thierry Reding wrote: > > > > > > > On Wed, Jun 19, 2013 at 02:00:45PM +0200, Laurent Pinchart wrote: > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Documentation/DocBook/drm.tmpl | 14 -- > > > > > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > > > > > > > > > > > diff --git a/Documentation/DocBook/drm.tmpl > > > > > > > > b/Documentation/DocBook/drm.tmpl index f9df3b8..738b727 100644 > > > > > > > > --- a/Documentation/DocBook/drm.tmpl > > > > > > > > +++ b/Documentation/DocBook/drm.tmpl > > > > > > > > @@ -186,11 +186,12 @@ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > DRIVER_HAVE_IRQDRIVER_IRQ_SHARED > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - DRIVER_HAVE_IRQ indicates whether the driver has > > > > > > > > an IRQ > > > > > > > > handler. The - DRM core will automatically > > > > > > > > register an > > > > > > > > interrupt handler when the - flag is set. > > > > > > > > DRIVER_IRQ_SHARED > > > > > > > > indicates whether the device & - handler > > > > > > > > support > > > > > > > > shared > > > > > > > > IRQs (note that this is required of PCI - drivers). > > > > > > > > + DRIVER_HAVE_IRQ indicates whether the driver has > > > > > > > > an IRQ > > > > > > > > handler + managed by the DRM Core. The core will > > > > > > > > support > > > > > > > > simple IRQ handler + installation when the flag is > > > > > > > > set. > > > > > > > > The > > > > > > > > installation process is + described in > > > > > > > linkend="drm-irq-registration"/>. + > > > > > > > > DRIVER_IRQ_SHARED indicates whether the device & > > > > > > > > handler + > > > > > > > > > > > > > > > > support shared IRQs (note that this is required of PCI > > > > > > > > drivers).> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -344,7 +345,8 @@ char *date; > > > > > > > > > > > > > > > >The DRM core tries to facilitate IRQ handler > > > > > > > > registration > > > > > > > >and > > > > > > > >unregistration by providing > > > > > > > >drm_irq_install and > > > > > > > >drm_irq_uninstall functions. > > > > > > > > Those > > > > > > > >functions only > > > > > > > > > > > > > > > > - support a single interrupt per device. > > > > > > > > + support a single interrupt per device, devices that > > > > > > > > use > > > > > > > > more > > > > > > > > than one + IRQs need to be handled manually. > > > > > > > > > > > > > > Perhaps this should mention that if you handle IRQ installation > > > > > > > manually > > > > > > > you also need to manually set drm->irq_enabled = 1, as otherwise > > > > > > > things > > > > > > > like DRM_IOCTL_WAIT_VBLANK won't work properly. > > > > > > > > > > > > That's only needed if DRIVER_HAVE_IRQ is set, otherwise the > > > > > > drm_wait_vblank() function skips the irq_enabled check. > > > > > > > > > > Not quite. There's another check for dev->irq_enabled in the > > > > > DRM_WAIT_ON() so it'll never actually block. > > > > > > > > Indeed. > > > > > > > > > Arguably this could be solved by making the DRM_WAIT_ON() condition > > > > > drop the > > > > > !dev->irq_enabled in case DRIVER_HAVE_IRQ isn't set. > > > > > > > > Or we could force drivers to set drm->irq_enabled, I'm fine with that > > > > as well. > > > > I can fix the documentation if that would be the preferred solution. > > > > > > Thinking about it some more, the latter seems more appropriate. Consider > > > some driver that doesn't set DRIVER_HAVE_IRQ but also doesn't initialize > > > interrupts manually. If so it'll cause DRM_IOCTL_WAIT_VBLANK to block > > > indefinitely. > > > > > > On the other hand perhaps the check at the beginning of drm_wait_vblank > > > should be improved. Maybe make it something like this: > > > > > > if (drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) > > > if (!drm_dev_to_irq(dev)) > > > return -EINVAL; > > > > > > if (!dev->irq_enabled) > > > return -EINVAL; > > > > I think the vblank subsystem is ripe
PROBLEM: WARNING, plane B assertion failure
Can you please boot with drm.debug=0xe added to your kernel bootline, reproduce the issue and the attach the complete dmesg? Thanks, Daniel On Mon, Jun 24, 2013 at 4:35 AM, brian porter wrote: > BIOS Information > Vendor: Hewlett-Packard > Version: 361A0 Ver. F.11 > System Information > Manufacturer: Hewlett-Packard > Product Name: HP Mini > Version: F.11 > Wake-up Type: Power Switch > SKU Number: FW376UA#ABA > Family: 103C_5335KV > > 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GSE > Express Integrated Graphics Controller (rev 03) (prog-if 0 > 0 [VGA controller]) > Subsystem: Hewlett-Packard Company Device 361a > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr- Stepping- SERR- FastB2B- DisINTx- > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- > SERR- Latency: 0 > Interrupt: pin A routed to IRQ 16 > Region 0: Memory at fe98 (32-bit, non-prefetchable) [size=512K] > Region 1: I/O ports at dc80 [size=8] > Region 2: Memory at d000 (32-bit, prefetchable) [size=256M] > Region 3: Memory at fe94 (32-bit, non-prefetchable) [size=256K] > Expansion ROM at [disabled] > Capabilities: [90] MSI: Enable- Count=1/1 Maskable- 64bit- > Address: Data: > Capabilities: [d0] Power Management version 2 > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA > PME(D0-,D1-,D2-,D3hot-,D3cold-) > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- > Kernel driver in use: i915 > Kernel modules: i915 > > > [3.243456] [ cut here ] > [3.243541] WARNING: at drivers/gpu/drm/i915/intel_display.c:1274 > intel_disable_pipe+0x146/0x150 [i915]() > [3.243548] Hardware name: HP Mini > [3.243554] plane B assertion failure, should be off on pipe B but > is still active > [3.243559] Modules linked in: cfg80211 hp_wmi(+) sparse_keymap > iTCO_wdt iTCO_vendor_support rfkill uvcvideo coretemp > videobuf2_vmalloc videobuf2_memops videobuf2_core microcode videodev > usb_storage media snd_hda_intel(+) i915(+) evdev psmouse serio_raw > pcspkr snd_hda_codec lpc_ich i2c_i801 of_i2c snd_hwdep drm_kms_helper > wmi snd_pcm ssb snd_page_alloc mmc_core snd_timer drm snd pcmcia > soundcore battery pcmcia_core acpi_cpufreq mperf sky2 video thermal ac > intel_agp intel_gtt agpgart i2c_algo_bit i2c_core button processor > ext4 crc16 mbcache jbd2 sd_mod ata_generic pata_acpi ata_piix libata > ehci_pci uhci_hcd ehci_hcd scsi_mod usbcore usb_common > [3.243695] Pid: 117, comm: systemd-udevd Not tainted 3.9.7-1-ARCH #1 > [3.243701] Call Trace: > [3.243721] [] warn_slowpath_common+0x6c/0xa0 > [3.243788] [] ? intel_disable_pipe+0x146/0x150 [i915] > [3.243854] [] ? intel_disable_pipe+0x146/0x150 [i915] > [3.243867] [] warn_slowpath_fmt+0x33/0x40 > [3.243933] [] intel_disable_pipe+0x146/0x150 [i915] > [3.244000] [] i9xx_crtc_disable+0xec/0x1c0 [i915] > [3.244233] [] intel_crtc_disable+0x2e/0x110 [i915] > [3.244299] [] __intel_set_mode+0x242/0x880 [i915] > [3.244316] [] ? mutex_unlock+0xd/0x10 > [3.244347] [] ? drm_framebuffer_init+0x80/0x90 [drm] > [3.244417] [] intel_set_mode+0x23/0x40 [i915] > [3.244647] [] intel_get_load_detect_pipe+0x230/0x3a0 [i915] > [3.244724] [] intel_modeset_setup_hw_state+0x714/0x8b0 [i915] > [3.244796] [] intel_modeset_gem_init+0x20/0x30 [i915] > [3.244852] [] i915_driver_load+0xa86/0xcb0 [i915] > [3.245039] [] ? i915_switcheroo_set_state+0xa0/0xa0 [i915] > [3.245091] [] drm_get_pci_dev+0x13b/0x260 [drm] > [3.245152] [] i915_pci_probe+0x3a/0x90 [i915] > [3.245168] [] pci_device_probe+0x6f/0xb0 > [3.245184] [] driver_probe_device+0x79/0x360 > [3.245199] [] __driver_attach+0x71/0x80 > [3.245211] [] ? __device_attach+0x40/0x40 > [3.245223] [] bus_for_each_dev+0x47/0x80 > [3.245236] [] driver_attach+0x1e/0x20 > [3.245247] [] ? __device_attach+0x40/0x40 > [3.245258] [] bus_add_driver+0x1df/0x2a0 > [3.245402] [] ? pci_device_probe+0xb0/0xb0 > [3.245417] [] ? pci_device_probe+0xb0/0xb0 > [3.245429] [] driver_register+0x6a/0x130 > [3.245444] [] __pci_register_driver+0x32/0x40 > [3.245473] [] drm_pci_init+0xf5/0x100 [drm] > [3.245504] [] ? 0xf8809fff > [3.245568] [] i915_init+0x5e/0x60 [i915] > [3.245581] [] do_one_initcall+0x10a/0x150 > [3.245594] [] ? __blocking_notifier_call_chain+0x44/0x60 > [3.245609] [] load_module+0x19a6/0x2340 > [3.245627] [] sys_init_module+0x85/0xe0 > [3.245641] [] sysenter_do_call+0x12/0x28 > [3.245785] ---[ end trace 62583c49c20e738c ]--- -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] Bootup regression of v3.10-rc6 + SWIOTLB + Intel 4000.
Hey Dave, Chris, Imre,
Attached is a fix that makes v3.10-rc6 boot on Intel HD 4000 when SWIOTLB
bounce buffer is in usage. The SWIOTLB can only handle up to 512KB swath
of memory to create bounce buffers for and Imre's patch made it possible
to provide more than to the DMA API which caused it to fail with dma_map_sg.
Since this is rc7 time I did the less risky way of fixing it - by just
doing what said code did before 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
("drm/i915: create compact dma scatter lists for gem objects") was
introduced by using a check to see if SWIOTLB is enabled.
It is not the best fix but I figured the less risky.
drivers/gpu/drm/i915/i915_gem.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
I think that a better approach (in v3.11?) would be to do some form of
retry mechanism: (not compile tested, not run at all):
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b9d00dc..0f9079d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1110,8 +1110,12 @@ struct drm_i915_gem_object_ops {
* will therefore most likely be called when the object itself is
* being released or under memory pressure (where we attempt to
* reap pages for the shrinker).
+*
+* max is the maximum size an sg entry can be. Usually it is
+* PAGE_SIZE but if the backend (IOMMU) can deal with larger
+* then a larger value might be used as well.
*/
- int (*get_pages)(struct drm_i915_gem_object *);
+ int (*get_pages)(struct drm_i915_gem_object *, unsigned long max);
void (*put_pages)(struct drm_i915_gem_object *);
};
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7045f45..a29e7db 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1738,7 +1738,7 @@ i915_gem_shrink_all(struct drm_i915_private *dev_priv)
}
static int
-i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
+i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj, unsigned long
max)
{
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
int page_count, i;
@@ -1809,7 +1809,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
continue;
}
#endif
- if (!i || page_to_pfn(page) != last_pfn + 1) {
+ if ((!i || (page_to_pfn(page) != last_pfn + 1)) && (sg->length
< max)) {
if (i)
sg = sg_next(sg);
st->nents++;
@@ -1847,7 +1847,7 @@ err_pages:
* or as the object is itself released.
*/
int
-i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
+i915_gem_object_get_pages(struct drm_i915_gem_object *obj, unsigned int max)
{
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
const struct drm_i915_gem_object_ops *ops = obj->ops;
@@ -1863,7 +1863,7 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
BUG_ON(obj->pages_pin_count);
- ret = ops->get_pages(obj);
+ ret = ops->get_pages(obj, max);
if (ret)
return ret;
@@ -2942,7 +2942,12 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
u32 size, fence_size, fence_alignment, unfenced_alignment;
bool mappable, fenceable;
int ret;
+ static unsigned int max_size = 4 * 1024 * 1024; /* 4MB */
+#ifdef CONFIG_SWIOTLB
+ if (swiotlb_nr_tbl())
+ max_size = PAGE_SIZE;
+#endif
fence_size = i915_gem_get_gtt_size(dev,
obj->base.size,
obj->tiling_mode);
@@ -2972,8 +2977,8 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
DRM_ERROR("Attempting to bind an object larger than the
aperture\n");
return -E2BIG;
}
-
- ret = i915_gem_object_get_pages(obj);
+ retry:
+ ret = i915_gem_object_get_pages(obj, max_size);
if (ret)
return ret;
@@ -3015,6 +3020,10 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object
*obj,
if (ret) {
i915_gem_object_unpin_pages(obj);
drm_mm_put_block(node);
+ if (max_size > PAGE_SIZE) {
+ max_size >> 1;
+ goto retry;
+ }
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index dc53a52..8101387 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -230,7 +230,8 @@ struct dma_buf *i915_gem_prime_export(struct drm_device
*dev,
return dma_buf_export(obj, &i915_dmabuf_ops, obj->base.size, flags);
}
-static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_objec
[PATCH] drm/i915: make compact dma scatter lists creation work with SWIOTLB backend.
Git commit 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
("drm/i915: create compact dma scatter lists for gem objects") makes
certain assumptions about the under laying DMA API that are not always
correct.
On a ThinkPad X230 with an Intel HD 4000 with Xen during the bootup
I see:
[drm:intel_pipe_set_base] *ERROR* pin & fence failed
[drm:intel_crtc_set_config] *ERROR* failed to set mode on [CRTC:3], err = -28
Bit of debugging traced it down to dma_map_sg failing (in
i915_gem_gtt_prepare_object) as some of the SG entries were huge (3MB).
That unfortunately are sizes that the SWIOTLB is incapable of handling -
the maximum it can handle is a an entry of 512KB of virtual contiguous
memory for its bounce buffer. (See IO_TLB_SEGSIZE).
Previous to the above mention git commit the SG entries were of 4KB, and
the code introduced by above git commit squashed the CPU contiguous PFNs
in one big virtual address provided to DMA API.
This patch is a simple semi-revert - were we emulate the old behavior
if we detect that SWIOTLB is online. If it is not online then we continue
on with the new compact scatter gather mechanism.
An alternative solution would be for the the '.get_pages' and the
i915_gem_gtt_prepare_object to retry with smaller max gap of the
amount of PFNs that can be combined together - but with this issue
discovered during rc7 that might be too risky.
Reported-and-Tested-by: Konrad Rzeszutek Wilk
CC: Chris Wilson
CC: Imre Deak
CC: Daniel Vetter
CC: David Airlie
CC:
Signed-off-by: Konrad Rzeszutek Wilk
---
drivers/gpu/drm/i915/i915_gem.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 970ad17..7045f45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1801,7 +1801,14 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp &= ~(__GFP_IO | __GFP_WAIT);
}
-
+#ifdef CONFIG_SWIOTLB
+ if (swiotlb_nr_tbl()) {
+ st->nents++;
+ sg_set_page(sg, page, PAGE_SIZE, 0);
+ sg = sg_next(sg);
+ continue;
+ }
+#endif
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
@@ -1812,8 +1819,10 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
}
last_pfn = page_to_pfn(page);
}
-
- sg_mark_end(sg);
+#ifdef CONFIG_SWIOTLB
+ if (!swiotlb_nr_tbl())
+#endif
+ sg_mark_end(sg);
obj->pages = st;
if (i915_gem_object_needs_bit17_swizzle(obj))
--
1.8.1.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/23] modetest: Add a command line parameter to set properties
On Fri, Jun 14, 2013 at 11:34:42PM +0200, Laurent Pinchart wrote:
> The -w parameter can be used to set a property value from the command
> line, using the target object ID and the property name.
>
> Signed-off-by: Laurent Pinchart
> ---
> tests/modetest/modetest.c | 108
> +-
> 1 file changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 778af62..858d480 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1008,9 +1082,20 @@ static int parse_plane(struct plane_arg *p, const char
> *arg)
> return 0;
> }
>
> +static int parse_property(struct property_arg *p, const char *arg)
> +{
> + if (sscanf(arg, "%d:%32[^:]:%" SCNu64, &p->obj_id, p->name, &p->value)
> != 3)
nit: could use stringification to get rid of the magic number 32 here.
I didn't spot any real problems in the series. But I must admit that I
mainly just glanced at most of the changes in since many of the diffs
are a bit hard to read.
I also gave it a quick try using sprites and setting a few modes. And I
found a bug in i915 while doing that, so clearly it has already proved
useful ;)
> + return -1;
> +
> + p->obj_type = 0;
> + p->name[DRM_PROP_NAME_LEN] = '\0';
> +
> + return 0;
> +}
> +
> static void usage(char *name)
> {
> - fprintf(stderr, "usage: %s [-cdefMmPpsv]\n", name);
> + fprintf(stderr, "usage: %s [-cdefMmPpsvw]\n", name);
>
> fprintf(stderr, "\n Query options:\n\n");
> fprintf(stderr, "\t-c\tlist connectors\n");
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression introduced by 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4 ("drm/i915: create compact dma scatter lists for gem objects") Was:Re: i915 mapping large (3MB) scatter list, hitting limits on ce
On Sat, Jun 22, 2013 at 03:22:59PM +0100, Chris Wilson wrote:
> On Fri, Jun 21, 2013 at 10:03:43PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 21, 2013 at 03:28:28PM -0400, Konrad Rzeszutek Wilk wrote:
> > > Hey,
> >
> > CC-ing Imre,
> >
> > Imre, your patch 90797e6d1ec0dfde6ba62a48b9ee3803887d6ed4
> > ("drm/i915: create compact dma scatter lists for gem objects") is the cause
> > of the regression.
> >
> > If I revert your patch it boots fine without any trouble.
> >
> > I am not entirely sure why that is - as I added some debug code in
> > lib/swiotlb.c to trigger when it can't find 3MB area (which
> > is what I thought initially was the issue) - but none of the debug
> > code seems to be hit.
> >
> > Any thoughts?
>
> You should be hitting drivers/iommu/intel-iommu.c for the dma
> translation. It looks like the contiguous 3MiB segment will be special
> as it is the first sg that __domain_mapping() will attempt to allocate a
> superpage (2MiB) for. What goes wrong at point, I am not sure, but I
> would suggest peppering intel-iommu.c with printk to track down the error.
I figured it out. The issue was that I am backed by the SWIOTLB which
can only allocate up to 128*4K chunks of contingous bounce buffer
(see IO_TLB_SEGSIZE) - which means it can only setup up to 512kB buffers.
While one of the SG entries tries to give it one past that size (3MB).
The change Imre introduced assume that the CPU addresses (virtual) are
the same as the bus addresses. That is correct in most platforms, but some
(for example when booting a Xen PV guest with i915 as PCI passthrough)
the virt_to_phys() values != bus address. Which means that the nice check of:
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
st->nents++;
sg_set_page(sg, page, PAGE_SIZE, 0);
} else {
sg->length += PAGE_SIZE;
}
is too simplistic. What it ought to do is consult the DMA API whether the
next PFN (page_to_pfn(page)) is really contingous in the DMA space. And also
that it does got past the DMA mask for said device.
Unfortunatly such calls do not exist. Those checks are all done when
dma_map_page_* is done (which is where it failed for me).
The best (for an rc7 stage) fix I came up with is to revert just a bit
of the old behavior and still retain the sg compact code behavior.
See following patch:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 970ad17..9edd2eb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1801,7 +1801,12 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
gfp |= __GFP_NORETRY | __GFP_NOWARN | __GFP_NO_KSWAPD;
gfp &= ~(__GFP_IO | __GFP_WAIT);
}
-
+ if (swiotlb_nr_tbl()) {
+ st->nents++;
+ sg_set_page(sg, page, PAGE_SIZE, 0);
+ sg = sg_next(sg);
+ continue;
+ }
if (!i || page_to_pfn(page) != last_pfn + 1) {
if (i)
sg = sg_next(sg);
@@ -1813,7 +1818,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object
*obj)
last_pfn = page_to_pfn(page);
}
- sg_mark_end(sg);
+ if (!swiotlb_nr_tbl())
+ sg_mark_end(sg);
obj->pages = st;
if (i915_gem_object_needs_bit17_swizzle(obj))
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 64695] Enabling both MLAA and MLAA color 2D crashes Gnome Shell on Cayman (6950)
https://bugs.freedesktop.org/show_bug.cgi?id=64695 --- Comment #3 from Brian Paul --- Created attachment 81345 --> https://bugs.freedesktop.org/attachment.cgi?id=81345&action=edit Improved error handling for post-process code I don't know if this patch will help with your issue, but please give it a try. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
MTRR use in drivers
On Mon, Jun 24, 2013 at 6:58 AM, H. Peter Anvin wrote: > On 06/23/2013 01:54 PM, Dave Airlie wrote: breaking old boxes just because, is just going to get reverted when I get the first regression report that you broke old boxes. >>> >>> Not "just because", but *if* the choice is between breaking old boxes >>> and breaking new boxes I'll take the latter. >>> >> >> But Linus won't so your choice doesn't matter. > > I hate to break it to you, but we regress on ancient hardware all the > time. Optimization work gets done on modern machines, so the sweet spot > keeps moving. In particular, if supporting ancient hardware means > leaving a lot of performance on modern hardware on the table, we may > have to take that penalty. > > Fortunately, most of the time we don't have to. Big difference between optimization sweet-spot and deliberately breaking older systems. This is firmly in the second category, lots of Intel hardware stops being useable when MTRR and PAT isn't working, so much so we had to a warning to the driver when we detect such a thing. Dave.
PROBLEM: WARNING, plane B assertion failure
-- next part -- A non-text attachment was scrubbed... Name: dmesg_drmdebug Type: application/octet-stream Size: 69442 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130624/b4112bb4/attachment-0001.obj>
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 Alex Deucher changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |NOTABUG --- Comment #4 from Alex Deucher --- The driver is correct. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 --- Comment #3 from Marek Olšák --- The "RV530" driver string isn't actually the name of your GPU, it's a category that includes all RV53x GPUs, because the hw programming is the same for all of those from a Mesa driver's point of view. Same for the kernel. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
MTRR use in drivers
>> breaking old boxes just because, is just going to get reverted when I >> get the first regression report that you broke old boxes. >> > > Not "just because", but *if* the choice is between breaking old boxes > and breaking new boxes I'll take the latter. > But Linus won't so your choice doesn't matter. >> Andy Lutomirski just submitted a bunch of patches to clean up the DRM >> usage of mtrrs, they are in drm-next, afaik we no longer add them on >> PAT systems. > > Fantastic news. No issue, then, and no need to break anything. Granted I haven't tested Andy's patches on my AGP boxes, and I intend to, if they do cause any regressions he'll be working them out :-) Dave.
[Bug 66064] ATI Mobility FireGL V5250 hardware incorrectly matched with RV530 dri settings in r_300.dri
https://bugs.freedesktop.org/show_bug.cgi?id=66064 Francis Shim changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |--- --- Comment #2 from Francis Shim --- Thank you for the clarification and the link to the "NotebookCheck" site for further information. After doing further research, it was still unclear whether a mismatch still exists or not. We can rule out using RV350; however, now it appears that RV535 might be a better candidate than RV530. According to the ThinkWiki the graphics chipset is identified as M66 and may be identified as M56GL. See: http://www.thinkwiki.org/wiki/ATI_Mobility_FireGL_V5250 According to the Freedesktop Xorg DRI site the M66 graphics set is matched with RV535. See: http://dri.freedesktop.org/wiki/ATIRadeon/ Could someone check this out, please? -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
MTRR use in drivers
>>> Why do you care about performance when PAT is disabled? breaking old boxes just because, is just going to get reverted when I get the first regression report that you broke old boxes. Andy Lutomirski just submitted a bunch of patches to clean up the DRM usage of mtrrs, they are in drm-next, afaik we no longer add them on PAT systems. Dave. >> >> It will regress already slow boxes. We blacklist a LOT of P4s, PMs, etc and >> nobody ever took the pain to track down which ones of those actually have >> PAT+MTRR aliasing bugs. >> >> These boxes have boards like the Radeon X300, which needs either PAT or MTRR >> to not become unusable... >> > > We're talking hardware which is now many years old, but this is causing > very serious problems on real, modern hardware. As far as I understand > it, too, the blacklisting was precautionary (the only bug that I > personally know about is a performance bug, where WC would be > incorrectly converted to UC.) > > We need a way forward here. If it is the only way I think we would have > to sacrifice the old machines, but perhaps something can be worked out > (e.g. if PAT is disabled, fall back to MTRRs if available for ioremap_wc()). > > -hpa > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
[PULL] drm-intel-fixes
Hi Dave, One remaining regression fix for i915. I've left it in -fixes for more than a week since it's in tricky code, and it took us a few kernel releases to notice the regression at all. The fence leak is especially annoying on gen2/3 and will kill userspace there quickly. For extra paranoia we've added a WARN in -next to catch this, things seem to be solid now. Cheers, Daniel The following changes since commit 7d132055814ef17a6c7b69f342244c410a5e000f: Linux 3.10-rc6 (2013-06-15 11:51:07 -1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-06-24 for you to fetch changes up to 19b2dbde5732170a03bd82cc8bd442cf88d856f7: drm/i915: Restore fences after resume and GPU resets (2013-06-16 01:10:45 +0200) Chris Wilson (1): drm/i915: Restore fences after resume and GPU resets drivers/gpu/drm/i915/i915_drv.h |2 ++ drivers/gpu/drm/i915/i915_gem.c | 22 +- drivers/gpu/drm/i915/i915_suspend.c |1 + 3 files changed, 8 insertions(+), 17 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/7] mutex: add support for wound/wait style locks
Op 20-06-13 14:23, Ingo Molnar schreef: > * Maarten Lankhorst wrote: > >> Well they've helped me with some of the changes and contributed some >> code and/or fixes, but if acked-by is preferred I'll use that.. > Such contributions can be credited in the changelog, and/or copyright > notices, and/or the code itself. The signoff chain on the other hand is > strictly defined as a 'route the patch took', with a single point of > origin, the main author. See Documentation/SubmittingPatches, pt 12. > > [ A signoff chain _can_ signal multi-authored code where the code got > written by someone and then further fixed/developed by someone else - > who adds a SOB to the end - but in that case I expect to get the patch > from the last person in the signoff chain. ] > > Thanks, > > Ingo Is this better? I added some more to the changelog entry, clarified ttm and fixed the sob's. 8< Wound/wait mutexes are used when other multiple lock acquisitions of a similar type can be done in an arbitrary order. The deadlock handling used here is called wait/wound in the RDBMS literature: The older tasks waits until it can acquire the contended lock. The younger tasks needs to back off and drop all the locks it is currently holding, i.e. the younger task is wounded. For full documentation please read Documentation/ww-mutex-design.txt. Changes since RFC patch v1: - Updated to use atomic_long instead of atomic, since the reservation_id was a long. - added mutex_reserve_lock_slow and mutex_reserve_lock_intr_slow - removed mutex_locked_set_reservation_id (or w/e it was called) Changes since RFC patch v2: - remove use of __mutex_lock_retval_arg, add warnings when using wrong combination of mutex_(,reserve_)lock/unlock. Changes since v1: - Add __always_inline to __mutex_lock_common, otherwise reservation paths can be triggered from normal locks, because __builtin_constant_p might evaluate to false for the constant 0 in that case. Tests for this have been added in the next patch. - Updated documentation slightly. Changes since v2: - Renamed everything to ww_mutex. (mlankhorst) - Added ww_acquire_ctx and ww_class. (mlankhorst) - Added a lot of checks for wrong api usage. (mlankhorst) - Documentation updates. (danvet) Changes since v3: - Small documentation fixes (robclark) - Memory barrier fix (danvet) Changes since v4: - Remove ww_mutex_unlock_single and ww_mutex_lock_single. - Rename ww_mutex_trylock_single to ww_mutex_trylock. - Remove separate implementations of ww_mutex_lock_slow*, normal functions can be used. Inline versions still exist for extra debugging. - Cleanup unneeded memory barriers, add comment to the remaining smp_mb(). Changes since v5: - Clarify TTM -> TTM graphics subsystem References: https://lwn.net/Articles/548909/ Acked-by: Daniel Vetter Acked-by: Rob Clark Signed-off-by: Maarten Lankhorst --- Documentation/ww-mutex-design.txt | 344 include/linux/mutex-debug.h | 1 + include/linux/mutex.h | 355 +- kernel/mutex.c| 318 -- lib/debug_locks.c | 2 + 5 files changed, 1003 insertions(+), 17 deletions(-) create mode 100644 Documentation/ww-mutex-design.txt diff --git a/Documentation/ww-mutex-design.txt b/Documentation/ww-mutex-design.txt new file mode 100644 index 000..8a112dc --- /dev/null +++ b/Documentation/ww-mutex-design.txt @@ -0,0 +1,344 @@ +Wait/Wound Deadlock-Proof Mutex Design +== + +Please read mutex-design.txt first, as it applies to wait/wound mutexes too. + +Motivation for WW-Mutexes +- + +GPU's do operations that commonly involve many buffers. Those buffers +can be shared across contexts/processes, exist in different memory +domains (for example VRAM vs system memory), and so on. And with +PRIME / dmabuf, they can even be shared across devices. So there are +a handful of situations where the driver needs to wait for buffers to +become ready. If you think about this in terms of waiting on a buffer +mutex for it to become available, this presents a problem because +there is no way to guarantee that buffers appear in a execbuf/batch in +the same order in all contexts. That is directly under control of +userspace, and a result of the sequence of GL calls that an application +makes. Which results in the potential for deadlock. The problem gets +more complex when you consider that the kernel may need to migrate the +buffer(s) into VRAM before the GPU operates on the buffer(s), which +may in turn require evicting some other buffers (and you don't want to +evict other buffers which are already queued up to the GPU), but for a +simplified understanding of the problem you can ignore this. + +The algorithm that the TTM graphics subsystem came up with for dealing with +this problem is quite simple. For each group
[PATCH] drm/prime: replace NULL with error value in drm_prime_pages_to_sg
From: YoungJun Cho
Instead of NULL, error value is casted with ERR_PTR() for
drm_prime_pages_to_sg() and IS_ERR_OR_NULL() macro is replaced
with IS_ERR() macro for drm_gem_map_dma_buf().
Signed-off-by: YoungJun Cho
Signed-off-by: Seung-Woo Kim
Signed-off-by: Kyungmin Park
---
drivers/gpu/drm/drm_prime.c |8 +---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index f1699e9..a47eab4 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -74,7 +74,7 @@ static struct sg_table *drm_gem_map_dma_buf(struct
dma_buf_attachment *attach,
sgt = obj->dev->driver->gem_prime_get_sg_table(obj);
- if (!IS_ERR_OR_NULL(sgt)) {
+ if (!IS_ERR(sgt)) {
if (!dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir)) {
sg_free_table(sgt);
kfree(sgt);
@@ -417,8 +417,10 @@ struct sg_table *drm_prime_pages_to_sg(struct page
**pages, int nr_pages)
int ret;
sg = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
- if (!sg)
+ if (!sg) {
+ ret = -ENOMEM;
goto out;
+ }
ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
nr_pages << PAGE_SHIFT, GFP_KERNEL);
@@ -428,7 +430,7 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages,
int nr_pages)
return sg;
out:
kfree(sg);
- return NULL;
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL(drm_prime_pages_to_sg);
--
1.7.4.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Improve manual IRQ installation documentation
On Sat, Jun 22, 2013 at 02:10:59PM +0200, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart Hm, a bit a thin commit message, but changes look good. So with the commit message patched up this is Reviewed-by: Daniel Vetter > --- > Documentation/DocBook/drm.tmpl | 118 > - > 1 file changed, 70 insertions(+), 48 deletions(-) > > Changes since v1: > > - Document manual IRQ registration > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 91ee107..a608094 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -186,11 +186,12 @@ > > DRIVER_HAVE_IRQDRIVER_IRQ_SHARED > > - DRIVER_HAVE_IRQ indicates whether the driver has an IRQ > handler. The > - DRM core will automatically register an interrupt handler when > the > - flag is set. DRIVER_IRQ_SHARED indicates whether the device > & > - handler support shared IRQs (note that this is required of PCI > - drivers). > + DRIVER_HAVE_IRQ indicates whether the driver has an IRQ handler > + managed by the DRM Core. The core will support simple IRQ > handler > + installation when the flag is set. The installation process is > + described in . > + DRIVER_IRQ_SHARED indicates whether the device & > handler > + support shared IRQs (note that this is required of PCI > drivers). > > > > @@ -344,50 +345,71 @@ char *date; >The DRM core tries to facilitate IRQ handler registration and >unregistration by providing drm_irq_install > and >drm_irq_uninstall functions. Those functions > only > - support a single interrupt per device. > - > - > - > - Both functions get the device IRQ by calling > - drm_dev_to_irq. This inline function will > call a > - bus-specific operation to retrieve the IRQ number. For platform > devices, > - platform_get_irq(..., 0) is used to retrieve > the > - IRQ number. > - > - > - drm_irq_install starts by calling the > - irq_preinstall driver operation. The > operation > - is optional and must make sure that the interrupt will not get > fired by > - clearing all pending interrupt flags or disabling the interrupt. > - > - > - The IRQ will then be requested by a call to > - request_irq. If the DRIVER_IRQ_SHARED driver > - feature flag is set, a shared (IRQF_SHARED) IRQ handler will be > - requested. > - > - > - The IRQ handler function must be provided as the mandatory > irq_handler > - driver operation. It will get passed directly to > - request_irq and thus has the same prototype > as all > - IRQ handlers. It will get called with a pointer to the DRM device > as the > - second argument. > - > - > - Finally the function calls the optional > - irq_postinstall driver operation. The > operation > - usually enables interrupts (excluding the vblank interrupt, which > is > - enabled separately), but drivers may choose to enable/disable > interrupts > - at a different time. > - > - > - drm_irq_uninstall is similarly used to > uninstall an > - IRQ handler. It starts by waking up all processes waiting on a > vblank > - interrupt to make sure they don't hang, and then calls the optional > - irq_uninstall driver operation. The > operation > - must disable all hardware interrupts. Finally the function frees > the IRQ > - by calling free_irq. > + support a single interrupt per device, devices that use more than > one > + IRQs need to be handled manually. > > + > + Managed IRQ Registration > + > +Both the drm_irq_install and > + drm_irq_uninstall functions get the device IRQ > by > + calling drm_dev_to_irq. This inline function > will > + call a bus-specific operation to retrieve the IRQ number. For > platform > + devices, platform_get_irq(..., 0) is used to > + retrieve the IRQ number. > + > + > +drm_irq_install starts by calling the > +irq_preinstall driver operation. The > operation > +is optional and must make sure that the interrupt will not get > fired by > +clearing all pending interrupt flags or disabling the interrupt. > + > + > +The IRQ will then be requested by a call to > +request_irq. If the DRIVER_IRQ_SHARED driver > +feature flag is set, a shared (IRQF_SHARED) IRQ han
Re: [Intel-gfx] [PATCH] drm/i915: move i915_trace_irq_get() out of the tracing macro
On Fri, Jun 21, 2013 at 02:51:07PM +0200, Sebastian Andrzej Siewior wrote: > On 06/21/2013 01:08 PM, Chris Wilson wrote: > > On Fri, Jun 21, 2013 at 12:15:53PM +0200, Sebastian Andrzej Siewior wrote: > >> There is a report on RT about "BUG: scheduling while atomic" because the > >> sleeping lock is taken in tracing context. This patch simply moves > >> locking operation out of the tracing macro. > > > > No. This enables the IRQ, as well as making a number of > > very expensively serialised read, unconditionally. > > Ach in case CONFIG_TRACING is enabled but the tracepoint itself is > disabled. In that case it is probably best to drop this tracepoint from > -RT. Iirc when we've last discussed this with Thomas Gleixner he mentioned that there's a setup/teardown hook when enabling/disabling a tracepoint. We could use that to enable/disable interrupts. Needs a notch of logic rework though since the auto-irq disable code needs adjusting ... And on a 2nd look that trace_irq_seqno logic looks a bit broken anyway. It's somewhere on my todo list, but patches very much welcome. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm: add hotspot support for cursors.
On Thu, Jun 20, 2013 at 11:48:52AM +1000, Dave Airlie wrote:
> From: Dave Airlie
>
> So it looks like for virtual hw cursors on QXL we need to inform
> the "hw" device what the cursor hotspot parameters are. This
> makes sense if you think the host has to draw the cursor and interpret
> clicks from it. However the current modesetting interface doesn't support
> passing the hotspot information from userspace.
>
> This implements a new cursor ioctl, that takes the hotspot info as well,
> userspace can try calling the new interface and if it -ENOSYS, can just
> fallback to the old non-hotspot interface.
This needs to be updated since your patch implements the fallback to the
old interface transparently already. Otherwise looks sane.
Reviewed-by: Daniel Vetter
>
> Signed-off-by: Dave Airlie
> ---
> drivers/gpu/drm/drm_crtc.c | 35 +--
> drivers/gpu/drm/drm_drv.c | 1 +
> include/drm/drm_crtc.h | 5 +
> include/uapi/drm/drm.h | 1 +
> include/uapi/drm/drm_mode.h | 13 +
> 5 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index e7e9242..cc9eada 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -2099,10 +2099,10 @@ out:
> return ret;
> }
>
> -int drm_mode_cursor_ioctl(struct drm_device *dev,
> - void *data, struct drm_file *file_priv)
> +static int drm_mode_cursor_common(struct drm_device *dev,
> + struct drm_mode_cursor2 *req,
> + struct drm_file *file_priv)
> {
> - struct drm_mode_cursor *req = data;
> struct drm_mode_object *obj;
> struct drm_crtc *crtc;
> int ret = 0;
> @@ -2122,13 +2122,17 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
>
> mutex_lock(&crtc->mutex);
> if (req->flags & DRM_MODE_CURSOR_BO) {
> - if (!crtc->funcs->cursor_set) {
> + if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
> ret = -ENXIO;
> goto out;
> }
> /* Turns off the cursor if handle is 0 */
> - ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
> - req->width, req->height);
> + if (crtc->funcs->cursor_set2)
> + ret = crtc->funcs->cursor_set2(crtc, file_priv,
> req->handle,
> + req->width, req->height,
> req->hot_x, req->hot_y);
> + else
> + ret = crtc->funcs->cursor_set(crtc, file_priv,
> req->handle,
> + req->width, req->height);
> }
>
> if (req->flags & DRM_MODE_CURSOR_MOVE) {
> @@ -2143,6 +2147,25 @@ out:
> mutex_unlock(&crtc->mutex);
>
> return ret;
> +
> +}
> +int drm_mode_cursor_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_cursor *req = data;
> + struct drm_mode_cursor2 new_req;
> +
> + memcpy(&new_req, req, sizeof(struct drm_mode_cursor));
> + new_req.hot_x = new_req.hot_y = 0;
> +
> + return drm_mode_cursor_common(dev, &new_req, file_priv);
> +}
> +
> +int drm_mode_cursor2_ioctl(struct drm_device *dev,
> +void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_cursor2 *req = data;
> + return drm_mode_cursor_common(dev, req, file_priv);
> }
>
> /* Original addfb only supported RGB formats, so figure out which one */
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9cc247f..99fcd7c 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -166,6 +166,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl,
> DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES,
> drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY,
> drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> + DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl,
> DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
> };
>
> #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index adb3f9b..093c030 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -339,6 +339,9 @@ struct drm_crtc_funcs {
> /* cursor controls */
> int (*cursor_set)(struct drm_crtc *crtc, struct drm_file *file_priv,
> uint32_t handle, uint32_t width, uint32_t height);
> + int (*cursor_set2)(struct drm_crtc *crtc, struct drm_file *file_priv,
> +uint32_t handle, uint32_t width, ui
Re: [PATCH] drm: Improve manual IRQ installation documentation
On Sat, Jun 22, 2013 at 12:56:46PM +0200, Thierry Reding wrote: > On Thu, Jun 20, 2013 at 04:55:03PM +0200, Daniel Vetter wrote: > > On Thu, Jun 20, 2013 at 01:00:36PM +0200, Thierry Reding wrote: > > > On Thu, Jun 20, 2013 at 12:46:21PM +0200, Laurent Pinchart wrote: > > > > Hi Thierry, > > > > > > > > On Thursday 20 June 2013 12:40:26 Thierry Reding wrote: > > > > > On Thu, Jun 20, 2013 at 12:17:25PM +0200, Laurent Pinchart wrote: > > > > > > On Thursday 20 June 2013 12:10:47 Thierry Reding wrote: > > > > > > > On Wed, Jun 19, 2013 at 02:00:45PM +0200, Laurent Pinchart wrote: > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > > > > > > > --- > > > > > > > > > > > > > > > > Documentation/DocBook/drm.tmpl | 14 -- > > > > > > > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > > > > > > > > > > > > > diff --git a/Documentation/DocBook/drm.tmpl > > > > > > > > b/Documentation/DocBook/drm.tmpl index f9df3b8..738b727 100644 > > > > > > > > --- a/Documentation/DocBook/drm.tmpl > > > > > > > > +++ b/Documentation/DocBook/drm.tmpl > > > > > > > > @@ -186,11 +186,12 @@ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > DRIVER_HAVE_IRQDRIVER_IRQ_SHARED > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - DRIVER_HAVE_IRQ indicates whether the driver has > > > > > > > > an IRQ > > > > > > > > handler. The - DRM core will automatically > > > > > > > > register an > > > > > > > > interrupt handler when the - flag is set. > > > > > > > > DRIVER_IRQ_SHARED > > > > > > > > indicates whether the device & - handler > > > > > > > > support > > > > > > > > shared > > > > > > > > IRQs (note that this is required of PCI - drivers). > > > > > > > > + DRIVER_HAVE_IRQ indicates whether the driver has > > > > > > > > an IRQ > > > > > > > > handler + managed by the DRM Core. The core will > > > > > > > > support > > > > > > > > simple IRQ handler + installation when the flag is > > > > > > > > set. > > > > > > > > The > > > > > > > > installation process is + described in > > > > > > > linkend="drm-irq-registration"/>. + > > > > > > > > DRIVER_IRQ_SHARED indicates whether the device & > > > > > > > > handler + > > > > > > > > > > > > > > > > support shared IRQs (note that this is required of PCI > > > > > > > > drivers).> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > @@ -344,7 +345,8 @@ char *date; > > > > > > > > > > > > > > > >The DRM core tries to facilitate IRQ handler > > > > > > > > registration > > > > > > > >and > > > > > > > >unregistration by providing > > > > > > > >drm_irq_install and > > > > > > > >drm_irq_uninstall functions. > > > > > > > > Those > > > > > > > >functions only > > > > > > > > > > > > > > > > - support a single interrupt per device. > > > > > > > > + support a single interrupt per device, devices that > > > > > > > > use > > > > > > > > more > > > > > > > > than one + IRQs need to be handled manually. > > > > > > > > > > > > > > Perhaps this should mention that if you handle IRQ installation > > > > > > > manually > > > > > > > you also need to manually set drm->irq_enabled = 1, as otherwise > > > > > > > things > > > > > > > like DRM_IOCTL_WAIT_VBLANK won't work properly. > > > > > > > > > > > > That's only needed if DRIVER_HAVE_IRQ is set, otherwise the > > > > > > drm_wait_vblank() function skips the irq_enabled check. > > > > > > > > > > Not quite. There's another check for dev->irq_enabled in the > > > > > DRM_WAIT_ON() so it'll never actually block. > > > > > > > > Indeed. > > > > > > > > > Arguably this could be solved by making the DRM_WAIT_ON() condition > > > > > drop the > > > > > !dev->irq_enabled in case DRIVER_HAVE_IRQ isn't set. > > > > > > > > Or we could force drivers to set drm->irq_enabled, I'm fine with that > > > > as well. > > > > I can fix the documentation if that would be the preferred solution. > > > > > > Thinking about it some more, the latter seems more appropriate. Consider > > > some driver that doesn't set DRIVER_HAVE_IRQ but also doesn't initialize > > > interrupts manually. If so it'll cause DRM_IOCTL_WAIT_VBLANK to block > > > indefinitely. > > > > > > On the other hand perhaps the check at the beginning of drm_wait_vblank > > > should be improved. Maybe make it something like this: > > > > > > if (drm_core_check_feature(dev, DRIVER_HAVE_IRQ)) > > > if (!drm_dev_to_irq(dev)) > > > return -EINVAL; > > > > > > if (!dev->irq_enabled) > > > return -EINVAL; > > > > I think the vblank subsystem is ripe
Re: PROBLEM: WARNING, plane B assertion failure
Can you please boot with drm.debug=0xe added to your kernel bootline, reproduce the issue and the attach the complete dmesg? Thanks, Daniel On Mon, Jun 24, 2013 at 4:35 AM, brian porter wrote: > BIOS Information > Vendor: Hewlett-Packard > Version: 361A0 Ver. F.11 > System Information > Manufacturer: Hewlett-Packard > Product Name: HP Mini > Version: F.11 > Wake-up Type: Power Switch > SKU Number: FW376UA#ABA > Family: 103C_5335KV > > 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GSE > Express Integrated Graphics Controller (rev 03) (prog-if 0 > 0 [VGA controller]) > Subsystem: Hewlett-Packard Company Device 361a > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- > ParErr- Stepping- SERR- FastB2B- DisINTx- > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- > SERR- Latency: 0 > Interrupt: pin A routed to IRQ 16 > Region 0: Memory at fe98 (32-bit, non-prefetchable) [size=512K] > Region 1: I/O ports at dc80 [size=8] > Region 2: Memory at d000 (32-bit, prefetchable) [size=256M] > Region 3: Memory at fe94 (32-bit, non-prefetchable) [size=256K] > Expansion ROM at [disabled] > Capabilities: [90] MSI: Enable- Count=1/1 Maskable- 64bit- > Address: Data: > Capabilities: [d0] Power Management version 2 > Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA > PME(D0-,D1-,D2-,D3hot-,D3cold-) > Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- > Kernel driver in use: i915 > Kernel modules: i915 > > > [3.243456] [ cut here ] > [3.243541] WARNING: at drivers/gpu/drm/i915/intel_display.c:1274 > intel_disable_pipe+0x146/0x150 [i915]() > [3.243548] Hardware name: HP Mini > [3.243554] plane B assertion failure, should be off on pipe B but > is still active > [3.243559] Modules linked in: cfg80211 hp_wmi(+) sparse_keymap > iTCO_wdt iTCO_vendor_support rfkill uvcvideo coretemp > videobuf2_vmalloc videobuf2_memops videobuf2_core microcode videodev > usb_storage media snd_hda_intel(+) i915(+) evdev psmouse serio_raw > pcspkr snd_hda_codec lpc_ich i2c_i801 of_i2c snd_hwdep drm_kms_helper > wmi snd_pcm ssb snd_page_alloc mmc_core snd_timer drm snd pcmcia > soundcore battery pcmcia_core acpi_cpufreq mperf sky2 video thermal ac > intel_agp intel_gtt agpgart i2c_algo_bit i2c_core button processor > ext4 crc16 mbcache jbd2 sd_mod ata_generic pata_acpi ata_piix libata > ehci_pci uhci_hcd ehci_hcd scsi_mod usbcore usb_common > [3.243695] Pid: 117, comm: systemd-udevd Not tainted 3.9.7-1-ARCH #1 > [3.243701] Call Trace: > [3.243721] [] warn_slowpath_common+0x6c/0xa0 > [3.243788] [] ? intel_disable_pipe+0x146/0x150 [i915] > [3.243854] [] ? intel_disable_pipe+0x146/0x150 [i915] > [3.243867] [] warn_slowpath_fmt+0x33/0x40 > [3.243933] [] intel_disable_pipe+0x146/0x150 [i915] > [3.244000] [] i9xx_crtc_disable+0xec/0x1c0 [i915] > [3.244233] [] intel_crtc_disable+0x2e/0x110 [i915] > [3.244299] [] __intel_set_mode+0x242/0x880 [i915] > [3.244316] [] ? mutex_unlock+0xd/0x10 > [3.244347] [] ? drm_framebuffer_init+0x80/0x90 [drm] > [3.244417] [] intel_set_mode+0x23/0x40 [i915] > [3.244647] [] intel_get_load_detect_pipe+0x230/0x3a0 [i915] > [3.244724] [] intel_modeset_setup_hw_state+0x714/0x8b0 [i915] > [3.244796] [] intel_modeset_gem_init+0x20/0x30 [i915] > [3.244852] [] i915_driver_load+0xa86/0xcb0 [i915] > [3.245039] [] ? i915_switcheroo_set_state+0xa0/0xa0 [i915] > [3.245091] [] drm_get_pci_dev+0x13b/0x260 [drm] > [3.245152] [] i915_pci_probe+0x3a/0x90 [i915] > [3.245168] [] pci_device_probe+0x6f/0xb0 > [3.245184] [] driver_probe_device+0x79/0x360 > [3.245199] [] __driver_attach+0x71/0x80 > [3.245211] [] ? __device_attach+0x40/0x40 > [3.245223] [] bus_for_each_dev+0x47/0x80 > [3.245236] [] driver_attach+0x1e/0x20 > [3.245247] [] ? __device_attach+0x40/0x40 > [3.245258] [] bus_add_driver+0x1df/0x2a0 > [3.245402] [] ? pci_device_probe+0xb0/0xb0 > [3.245417] [] ? pci_device_probe+0xb0/0xb0 > [3.245429] [] driver_register+0x6a/0x130 > [3.245444] [] __pci_register_driver+0x32/0x40 > [3.245473] [] drm_pci_init+0xf5/0x100 [drm] > [3.245504] [] ? 0xf8809fff > [3.245568] [] i915_init+0x5e/0x60 [i915] > [3.245581] [] do_one_initcall+0x10a/0x150 > [3.245594] [] ? __blocking_notifier_call_chain+0x44/0x60 > [3.245609] [] load_module+0x19a6/0x2340 > [3.245627] [] sys_init_module+0x85/0xe0 > [3.245641] [] sysenter_do_call+0x12/0x28 > [3.245785] ---[ end trace 62583c49c20e738c ]--- -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-
