Re: [Intel-gfx] [RFC 11/14] drm/i915: Enable MIPI display self refresh mode
On 5/29/2015 10:51 PM, Daniel Vetter wrote:
On Fri, May 29, 2015 at 04:07:03PM +0530, Gaurav K Singh wrote:
During enable sequence for MIPI encoder in command mode, enable
MIPI display self-refresh mode bit in Pipe Ctrl reg.
Signed-off-by: Gaurav K Singh
Signed-off-by: Yogesh Mohan Marimuthu
Signed-off-by: Shobhit Kumar
---
drivers/gpu/drm/i915/intel_display.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cab2ac8..fc84313 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include "intel_dsi.h"
/* Primary plane formats supported by all gen */
#define COMMON_PRIMARY_FORMATS \
@@ -2110,6 +2111,8 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_encoder *encoder;
+ struct intel_dsi *intel_dsi;
enum pipe pipe = crtc->pipe;
enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
pipe);
@@ -2154,6 +2157,18 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
return;
}
+ for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
+ if (encoder->type == INTEL_OUTPUT_DSI) {
+ intel_dsi = enc_to_intel_dsi(&encoder->base);
+ if (intel_dsi && (intel_dsi->operation_mode ==
+ INTEL_DSI_COMMAND_MODE)) {
+ val = val | PIPECONF_MIPI_DSR_ENABLE;
+ I915_WRITE(reg, val);
+ }
+ break;
+ }
+ }
When we have these kind of encoder/crtc state depencies we resolve them by
adding a bit of state to intel_crtc_state which is set as needed in the
encoder's compute_config callback. Then all you need here is
if (intel_state->dsi_self_refresh)
val |= PIPECONF_MIPI_DSR_ENABLE;
Also is that additional write really required?
-Daniel
Yes additional write is required. MIPI_DSR_ENABLE has to be written
first then followed
by pipe enable. if MIPI_DSR_ENABLE and pipe enable is done in same
write, observed
that the image from pipe is not sent to panel when issued mem write command.
Having a state variable instead of looping through the encoders
definitely looks good.
Need to find a place to update the state variable. I will get back on this.
+
I915_WRITE(reg, val | PIPECONF_ENABLE);
POSTING_READ(reg);
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915: Enable PSR by default.
Hi Matthew, here is the patch I've mentioned on irc today: http://cgit.freedesktop.org/~vivijim/drm-intel/commit/?h=psr_for_mjg59&id=83809492138f2395bfb12c19e6de916de64b9246 And I prepared this branch for now: http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=psr_for_mjg59 I'm not sending yet the patches because I still face the missed screen during boot that you had mentioned. As soon as I fixed it I'll submit everything. Thanks, Rodrigo. On Mon, Apr 20, 2015 at 7:43 AM, Vivi, Rodrigo wrote: > On Sat, 2015-04-18 at 08:27 +0100, Matthew Garrett wrote: >> On Mon, Apr 13, 2015 at 04:46:29PM -0700, Rodrigo Vivi wrote: >> > Another questions, >> > >> > Are you using powertop --auto-tune? >> > >> > If so, can you please try to repdoruce X slowness issue on these 2 >> > scenarios: >> > 1. without doing the powertop auto-tune and psr enabled. >> >> Ah! Yes, this is the problem. If runtime PM is enabled on the i915 PCI >> device (and the HDMI HDA device), things break. If it's disabled, >> everything works fine. I hope that helps narrow it down! >> > > Are you using external USB keyboard and mouse? I can just face this > slowness when using USB, if I don't use any USB everything is fine. If > switch all USB powertop scripts to "Bad" I also can use my system > reliably. This seems a bug in usb power management. Could you please > verify if this is the same that I'm facing here? > > One extra thing, could you confirm that you face this behaviour even > with i915.enable_psr=0 so Daniel can accept this last patch? > > Thank you very much, > Rodrigo. -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] panning crash
Dell Optiplex GX280 P4 32 bit Hyperthreading enabled 0F34 2.8GHz CPU 00:02.0 VGA compatible controller: Intel Corporation 82915G/GV/910GL Integrated Graphics Controller (rev 04) /etc/X11/xinit/xinitrc.d/setup contains: xrandr --fbmm 387x218 --fb 2560x1440 --output VGA1 --mode 1920x1440 --panning 2560x1440 Panning mouse out of initial CRTC desktop space into virtual desktop space and back in some installations causes crash, in others not. Can someone here tell if this is a kernel, server or other issue and not a driver issue from a look at Xorg.0.log? My suspicion is kernel as primary if not sole. Successes: Rawhide, kernel 4.1.0-0.rc7, driver 2.99.917, server 1.17.1, KDE5 Mageia 5, kernel 3.19.8, driver 2.99.917, server 1.16.4, KDE4 openSUSE 13.1, kernel 3.12.39, driver 2.99.917, server 1.15.99.903, KDE3 openSUSE 13.2, kernel 3.16.7, driver 2.99.917, server 1.17.1, KDE3 Failures: Fedora 21, kernel 4.0.4, driver 2.99.916, server 1.16.3, KDE4 Tumbleweed, kernel 4.0.4, driver 2.99.917, server 1.17.1, KDE3 Skipped testing due to unrelated issue (refuses mode 1920x1440 that xrandr claims available, uses 1600x1200 instead, and no crashing): Fedora 22, kernel 4.0.5, driver 2.99.917, server 1.17.1, KDE5 Logs: http://fm.no-ip.com/Tmp/Linux/Xorg/Igfx/panfault/ -- "The wise are known for their understanding, and pleasant words are persuasive." Proverbs 16:21 (New Living Translation) Team OS/2 ** Reg. Linux User #211409 ** a11y rocks! Felix Miata *** http://fm.no-ip.com/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm: Add decoding of i915 ioctls
On Thu, Jun 11, 2015 at 03:34:14PM +0200, Patrik Jakobsson wrote:
> On Thu, Jun 11, 2015 at 02:27:12AM +0300, Dmitry V. Levin wrote:
> > On Wed, Jun 10, 2015 at 02:45:24PM +0200, Patrik Jakobsson wrote:
> > > On Wed, Jun 10, 2015 at 01:35:35AM +0300, Dmitry V. Levin wrote:
> > > > On Tue, Jun 09, 2015 at 01:26:43PM +0200, Patrik Jakobsson wrote:
> > [...]
> > > > > +static int i915_setparam(struct tcb *tcp, const unsigned int code,
> > > > > long arg)
> > > > > +{
> > > > > + struct drm_i915_setparam param;
> > > > > +
> > > > > + if (exiting(tcp) || umove(tcp, arg, ¶m))
> > > > > + return 0;
> > > >
> > > > In this and other ioctl printers that unconditionally return 0 on exit,
> > > > wouldn't the caller treat it as an ioctl that hasn't been printed?
> > >
> > > Yes, seems like the exiting phase should return 1 if already handled in
> > > the
> > > entering phase. But changing it produces the same output for some reason.
> > > Not
> > > sure what's going on here.
> >
> > Isn't tcp->u_arg[2] printed twice, the first time decoded,
> > and the second time in hex?
>
> My mistake, it does print u_arg[2] in hex as well. Luckily they could all be
> moved to the exiting phase instead. Fixed in v2.
The common rule is to decode as early as possible, consequently, all
syscall arguments that could be decoded on entering syscall should be
decoded on entering rather than on exiting. All syscall arguments
of _IOW ioctl commands could be fully decoded on entering syscall.
--
ldv
pgp2D72UvJixD.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm: Add dispatcher and driver identification for DRM
On Thu, Jun 11, 2015 at 04:11:49PM +0200, Patrik Jakobsson wrote:
> On Thu, Jun 11, 2015 at 02:26:59AM +0300, Dmitry V. Levin wrote:
> > On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
> > > On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> > > > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
> > [...]
> > > > > +#define DRM_MAX_NAME_LEN 128
> > > > > +
> > > > > +inline int drm_is_priv(const unsigned int num)
> > > > > +{
> > > > > + return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > > > > + _IOC_NR(num) < DRM_COMMAND_END);
> > > > > +}
> > > > > +
> > > > > +static int drm_get_driver_name(struct tcb *tcp, char *name, size_t
> > > > > bufsize)
> > > > > +{
> > > > > + char path[PATH_MAX];
> > > > > + char link[PATH_MAX];
> > > > > + int ret;
> > > > > +
> > > > > + ret = getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1);
> > > > > + if (!ret)
> > > > > + return ret;
> > > > > +
> > > > > + snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver",
> > > > > + basename(path));
> > > > > +
> > > > > + ret = readlink(link, path, PATH_MAX - 1);
> > > > > + if (ret < 0)
> > > > > + return ret;
> > > > > +
> > > > > + path[ret] = '\0';
> > > > > + strncpy(name, basename(path), bufsize);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int drm_is_driver(struct tcb *tcp, const char *name)
> > > > > +{
> > > > > + char drv[DRM_MAX_NAME_LEN];
> > > > > + int ret;
> > > > > +
> > > > > + ret = drm_get_driver_name(tcp, drv, DRM_MAX_NAME_LEN);
> > > > > + if (ret)
> > > > > + return 0;
> > > > > +
> > > > > + return strcmp(name, drv) == 0;
> > > > > +}
> > > >
> > > > This interface will result to several getfdpath() calls per
> > > > ioctl_decode(). If the only purpose of drm_is_driver() is to help
> > > > finding
> > > > the most appropriate function, let's create a table of pairs
> > > > {driver name, function} and pass this table to a function that will do a
> > > > single getfdpath() call, calculate the driver name, and choose the right
> > > > function from the table.
> > >
> > > Yes I was thinking the same thing but it's a bit tricky. What I need is:
> > > fd -> path -> driver name. And fd -> path could change between ioctls. It
> > > is not
> > > a likely scenario but it's possible. I could get rid of the extra call in
> > > drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could
> > > also
> > > optimize path -> driver name with a table but I don't know how expensive
> > > those
> > > calls actually are. Not sure what would be the best solution here.
> >
> > drm_get_driver_name() is quite expensive as it does two readlink syscalls,
> > so it should be called at most once per ioctl_decode().
> >
> > Another method to achieve this is to change drm_get_driver_name() to return
> > basename(path) instead of return code, so that drm_ioctl() would call it
> > once and pass the result to strcmp calls:
> >
> > int
> > drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> > {
> > if (verbose(tcp) && drm_is_priv(code)) {
> > const char *driver = drm_get_driver_name(tcp);
> > if (!driver)
> > return 0;
> > if (!strcmp(driver, "i915"))
> > return drm_i915_ioctl(tcp, code, arg);
> > }
> > return 0;
> > }
>
> I misunderstood you. As you say, we shouldn't do a drm_get_driver_name() more
> than once (no matter how many drm drivers we are looking for) so your
> suggestion
> above would fix that. I was thinking about how to get rid of the extra call in
> drm_decode_number() (if we could somehow squash them together). But that would
> make things rather ugly. If ok with you we could just have the same approach
> in
> drm_decode_number() as above and live with the fact that we get two calls to
> drm_get_driver_name() per DRM device specific ioctl. One for
> drm_decode_number()
> and one for drm_ioctl().
This way we would end up with three drm_get_driver_name() calls per ioctl:
twice on entering syscall and once on exiting. Maybe we could cache
result of the first of these three calls somewhere?
--
ldv
pgpK6Q3_uykrE.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] drm/i915: prevent out of range pt in the PDE macros (take 2)
From: Paulo Zanoni
We tried to fix this in the following commit:
commit fdc454c1484a20e1345cf4e4d7a9feaee814147f
Author: Michel Thierry
Date: Tue Mar 24 15:46:19 2015 +
drm/i915: Prevent out of range pt in gen6_for_each_pde
but the static analyzer still complains that, just before we break due
to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an
iter value that is bigger than I915_PDES. Of course, this isn't really
a problem since no one uses pt outside the macro. Still, every single
new usage of the macro will create a new issue for us to mark as a
false possitive.
After the commit mentioned above we also created some new versions of
the macros, so they carry the same "problem".
In order to "solve" this "problem", let's leave the macro with a NULL
value for pt. So if somebody uses it, we're more likely to get a big
error message instead of some silent failure. I hope the static
analyzer won't complain about the new solution (I don't have a way to
check this!).
I know, the solution looks really ugly. I am hoping the reviewers will
help us decide if we prefer this patch or if we prefer to keep marking
things as false positives.
Cc: Michel Thierry
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_gem_gtt.h | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
I sent this as an RFC because I really don't know if complicating the
macro even more will help us in any way. I won't really be surprised
if I see NACKs on this patch, so don't hesitate if you want to.
Also, all I did was boot a Kernel with this patch and make sure it
shows the desktop. So consider this as untested, possibly broken.
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 0d46dd2..b202ca0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -352,7 +352,8 @@ struct i915_hw_ppgtt {
*/
#define gen6_for_each_pde(pt, pd, start, length, temp, iter) \
for (iter = gen6_pde_index(start); \
-pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \
+pt = iter < I915_PDES ? (pd)->page_table[iter] : NULL, \
+length > 0 && iter < I915_PDES; \
iter++, \
temp = ALIGN(start+1, 1 << GEN6_PDE_SHIFT) - start, \
temp = min_t(unsigned, temp, length), \
@@ -360,7 +361,8 @@ struct i915_hw_ppgtt {
#define gen6_for_all_pdes(pt, ppgtt, iter) \
for (iter = 0; \
-pt = ppgtt->pd.page_table[iter], iter < I915_PDES; \
+pt = iter < I915_PDES ? ppgtt->pd.page_table[iter] : NULL, \
+iter < I915_PDES; \
iter++)
static inline uint32_t i915_pte_index(uint64_t address, uint32_t pde_shift)
@@ -417,7 +419,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
*/
#define gen8_for_each_pde(pt, pd, start, length, temp, iter) \
for (iter = gen8_pde_index(start); \
-pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES;
\
+pt = iter < I915_PDES ? (pd)->page_table[iter] : NULL, \
+length > 0 && iter < I915_PDES;\
iter++,\
temp = ALIGN(start+1, 1 << GEN8_PDE_SHIFT) - start,\
temp = min(temp, length), \
@@ -425,7 +428,9 @@ static inline uint32_t gen6_pde_index(uint32_t addr)
#define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \
for (iter = gen8_pdpe_index(start); \
-pd = (pdp)->page_directory[iter], length > 0 && iter <
GEN8_LEGACY_PDPES; \
+pd = iter < GEN8_LEGACY_PDPES ?\
+ (pdp)->page_directory[iter] : NULL, \
+length > 0 && iter < GEN8_LEGACY_PDPES;\
iter++,\
temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT) - start, \
temp = min(temp, length), \
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: use effective_size for ringbuffer calculations
On Fri, Jun 12, 2015 at 08:55:09PM +0100, Dave Gordon wrote: > > However, that makes an incorrect assumption about the waiter. Given that > > the current code is written such that ringbuf->last_retired_head = > > request->postfix and that space is identical to the repeated > > calculation, what is your intention exactly? > > -Chris > > Three factors: > > * firstly, a preference: I find it logically simpler that there should > be one and only one piece of code that writes into ringbuf->space. One > doesn't then have to reason about whether two different calculations are > in fact equivalent. I find the opposite, since we compute how much space we want I think it is counter intuitive to suggest otherwise. You then need to assert that the computed space is enough. By saying if we wait until this request, we must have at least this space available and only using that space there is no implicit knowlege. > * secondly, for future proofing: although wait_request() now retires > only up to the waited-for request, that wasn't always the case, nor is > there any reason why it could not in future opportunistically retire > additional requests that have completed while it was waiting. Because there is a cost associated with every retirement. See above for why being explicit is clearer. > * thirdly, for correctness: using the function has the additional effect > of consuming the last_retired_head value set by retire_request. If this > is not done, a later call to intel_ring_space() may become confused, > with the result that 'head' (and therefore 'space') will be incorrectly > updated. Eh. The code is still strictly correct. The biggest issue is that we have multiple locations that decide how to interpret the amount of space generated by completing the request. However, we want to keep request retirement very simple since it is a hot function. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] (no subject)
Updated version split into two. The first tidies up the _ring_prepare() functions and removes the corner case where we might have had to wait twice; the second is a temporary workaround to solve a kernel OOPS that can occur if logical_ring_begin is called while the ringbuffer is not mapped because there's no current request. The latter will be superseded by the Anti-OLR patch series currently in review. But this helps with GuC submission, which is better than the execlist path at exposing the problematic case :( ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Don't wait twice in {__intel, logical}_ring_prepare()
In the case that the ringbuffer was near-full AND 'tail' was
near the end of the buffer, we could have ended up waiting twice:
once to gain ownership of the space between TAIL and the end
(which we just want to fill with padding, so as not to split a
single command sequence across the end of the ringbuffer), and
then again to get enough space for the new data that the caller
wants to add.
Now we just precalculate the total amount of space we'll need
*including* any for padding at the end of the ringbuffer and wait
for that much in one go.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c| 52 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 51 +++---
2 files changed, 50 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 454e836..a4bb5d3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -740,39 +740,22 @@ intel_logical_ring_advance_and_submit(struct
intel_ringbuffer *ringbuf,
execlists_context_queue(ring, ctx, ringbuf->tail, request);
}
-static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf,
- struct intel_context *ctx)
-{
- uint32_t __iomem *virt;
- int rem = ringbuf->size - ringbuf->tail;
-
- if (ringbuf->space < rem) {
- int ret = logical_ring_wait_for_space(ringbuf, ctx, rem);
-
- if (ret)
- return ret;
- }
-
- virt = ringbuf->virtual_start + ringbuf->tail;
- rem /= 4;
- while (rem--)
- iowrite32(MI_NOOP, virt++);
-
- ringbuf->tail = 0;
- intel_ring_update_space(ringbuf);
-
- return 0;
-}
-
static int logical_ring_prepare(struct intel_ringbuffer *ringbuf,
struct intel_context *ctx, int bytes)
{
+ int fill = 0;
int ret;
+ /*
+* If the request will not fit between 'tail' and the effective
+* size of the ringbuffer, then we need to pad the end of the
+* ringbuffer with NOOPs, then start the request at the top of
+* the ring. This increases the total size that we need to check
+* for by however much is left at the end of the ring ...
+*/
if (unlikely(ringbuf->tail + bytes > ringbuf->effective_size)) {
- ret = logical_ring_wrap_buffer(ringbuf, ctx);
- if (unlikely(ret))
- return ret;
+ fill = ringbuf->size - ringbuf->tail;
+ bytes += fill;
}
if (unlikely(ringbuf->space < bytes)) {
@@ -781,6 +764,21 @@ static int logical_ring_prepare(struct intel_ringbuffer
*ringbuf,
return ret;
}
+ if (unlikely(fill)) {
+ uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail;
+
+ /* tail should not have moved */
+ if (WARN_ON(fill != ringbuf->size - ringbuf->tail))
+ fill = ringbuf->size - ringbuf->tail;
+
+ do
+ iowrite32(MI_NOOP, virt++);
+ while ((fill -= 4) > 0);
+
+ ringbuf->tail = 0;
+ intel_ring_update_space(ringbuf);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a3406b2..5a1cd20 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2137,29 +2137,6 @@ static int ring_wait_for_space(struct intel_engine_cs
*ring, int n)
return 0;
}
-static int intel_wrap_ring_buffer(struct intel_engine_cs *ring)
-{
- uint32_t __iomem *virt;
- struct intel_ringbuffer *ringbuf = ring->buffer;
- int rem = ringbuf->size - ringbuf->tail;
-
- if (ringbuf->space < rem) {
- int ret = ring_wait_for_space(ring, rem);
- if (ret)
- return ret;
- }
-
- virt = ringbuf->virtual_start + ringbuf->tail;
- rem /= 4;
- while (rem--)
- iowrite32(MI_NOOP, virt++);
-
- ringbuf->tail = 0;
- intel_ring_update_space(ringbuf);
-
- return 0;
-}
-
int intel_ring_idle(struct intel_engine_cs *ring)
{
struct drm_i915_gem_request *req;
@@ -2197,12 +2174,19 @@ static int __intel_ring_prepare(struct intel_engine_cs
*ring,
int bytes)
{
struct intel_ringbuffer *ringbuf = ring->buffer;
+ int fill = 0;
int ret;
+ /*
+* If the request will not fit between 'tail' and the effective
+* size of the ringbuffer, then we need to pad the end of the
+* ringbuffer with NOOPs, then start the request at the top of
+* the ring. This increases the total size that we need to check
+* for by however much is left at the end of the ring ...
+
[Intel-gfx] [PATCH 2/2] drm/i915: Allocate OLR more safely (workaround until OLR goes away)
The original idea of preallocating the OLR was implemented in
> 9d773091 drm/i915: Preallocate next seqno before touching the ring
and the sequence of operations was to allocate the OLR, then wrap past
the end of the ring if necessary, then wait for space if necessary.
But subsequently intel_ring_begin() was refactored, in
> 304d695 drm/i915: Flush outstanding requests before allocating new seqno
to ensure that pending work that might need to be flushed used the old
and not the newly-allocated request. This changed the sequence to wrap
and/or wait, then allocate, although the comment still said
/* Preallocate the olr before touching the ring */
which was no longer true as intel_wrap_ring_buffer() touches the ring.
However, with the introduction of dynamic pinning, in
> 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
came the possibility that the ringbuffer might not be pinned to the GTT
or mapped into CPU address space when intel_ring_begin() is called. It
gets pinned when the request is allocated, so it's now important that
this comes *before* anything that can write into the ringbuffer, in this
case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer
happens not to be mapped, and (b) tail happens to be sufficiently close
to the end of the ring to trigger wrapping.
So the correct order is neither the original allocate-wait-pad-wait, nor
the subsequent wait-pad-wait-allocate, but wait-allocate-pad, avoiding
both the problems described in the two commits mentioned above. So this
commit moves the calls to i915_gem_request_alloc() into the middle of
{__intel,logical}_ring_prepare() rather than either before or after them.
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c| 12 +++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++-
2 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index a4bb5d3..3ef5fb6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -764,6 +764,13 @@ static int logical_ring_prepare(struct intel_ringbuffer
*ringbuf,
return ret;
}
+ /* Ensure we have a request before touching the ring */
+ if (!ringbuf->ring->outstanding_lazy_request) {
+ ret = i915_gem_request_alloc(ringbuf->ring, ctx);
+ if (ret)
+ return ret;
+ }
+
if (unlikely(fill)) {
uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail;
@@ -812,11 +819,6 @@ static int intel_logical_ring_begin(struct
intel_ringbuffer *ringbuf,
if (ret)
return ret;
- /* Preallocate the olr before touching the ring */
- ret = i915_gem_request_alloc(ring, ctx);
- if (ret)
- return ret;
-
ringbuf->space -= num_dwords * sizeof(uint32_t);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5a1cd20..ddf580d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2195,6 +2195,13 @@ static int __intel_ring_prepare(struct intel_engine_cs
*ring,
return ret;
}
+ /* Ensure we have a request before touching the ring */
+ if (!ringbuf->ring->outstanding_lazy_request) {
+ ret = i915_gem_request_alloc(ringbuf->ring,
ring->default_context);
+ if (ret)
+ return ret;
+ }
+
if (unlikely(fill)) {
uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail;
@@ -2228,11 +2235,6 @@ int intel_ring_begin(struct intel_engine_cs *ring,
if (ret)
return ret;
- /* Preallocate the olr before touching the ring */
- ret = i915_gem_request_alloc(ring, ring->default_context);
- if (ret)
- return ret;
-
ring->buffer->space -= num_dwords * sizeof(uint32_t);
return 0;
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: use effective_size for ringbuffer calculations
On 12/06/15 19:12, Chris Wilson wrote: > On Fri, Jun 12, 2015 at 06:09:07PM +0100, Dave Gordon wrote: >> When calculating the available space in a ringbuffer, we should >> use the effective_size rather than the true size of the ring. >> >> v2: rebase to latest drm-intel-nightly >> v3: rebase to latest drm-intel-nightly >> >> Signed-off-by: Dave Gordon >> --- >> drivers/gpu/drm/i915/intel_lrc.c|5 +++-- >> drivers/gpu/drm/i915/intel_ringbuffer.c |9 ++--- >> 2 files changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c >> b/drivers/gpu/drm/i915/intel_lrc.c >> index 9b74ffa..454e836 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -699,7 +699,7 @@ static int logical_ring_wait_for_space(struct >> intel_ringbuffer *ringbuf, >> >> /* Would completion of this request free enough space? */ >> space = __intel_ring_space(request->postfix, ringbuf->tail, >> - ringbuf->size); >> + ringbuf->effective_size); >> if (space >= bytes) >> break; >> } >> @@ -711,7 +711,8 @@ static int logical_ring_wait_for_space(struct >> intel_ringbuffer *ringbuf, >> if (ret) >> return ret; >> >> -ringbuf->space = space; >> +/* Update ring space after wait+retire */ >> +intel_ring_update_space(ringbuf); > > Does the function not do what it says on the tin? At least make it seem > like you are explaining your reasoning, not documenting the following > function. > > /* > * Having waited for the request, query the HEAD of most recent retired > * request and use that for our space calcuations. > */ That's what the comment means; the important bit is mentioning "retire", since it's not explicitly called from here but only via wait_request(). We could say, /* * After waiting, at least one request must have completed * and been retired, so make sure that the ringbuffer's * space calculations are up to date */ intel_ring_update_space(ringbuf); BUG_ON(ringbuf->space < bytes); > However, that makes an incorrect assumption about the waiter. Given that > the current code is written such that ringbuf->last_retired_head = > request->postfix and that space is identical to the repeated > calculation, what is your intention exactly? > -Chris Three factors: * firstly, a preference: I find it logically simpler that there should be one and only one piece of code that writes into ringbuf->space. One doesn't then have to reason about whether two different calculations are in fact equivalent. * secondly, for future proofing: although wait_request() now retires only up to the waited-for request, that wasn't always the case, nor is there any reason why it could not in future opportunistically retire additional requests that have completed while it was waiting. * thirdly, for correctness: using the function has the additional effect of consuming the last_retired_head value set by retire_request. If this is not done, a later call to intel_ring_space() may become confused, with the result that 'head' (and therefore 'space') will be incorrectly updated. .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rework order of operations in {__intel, logical}_ring_prepare()
On Fri, Jun 12, 2015 at 07:54:17PM +0100, Dave Gordon wrote: > On 12/06/15 19:05, Chris Wilson wrote: > > On Fri, Jun 12, 2015 at 06:09:08PM +0100, Dave Gordon wrote: > >> The original idea of preallocating the OLR was implemented in > >> > >>> 9d773091 drm/i915: Preallocate next seqno before touching the ring > >> > >> and the sequence of operations was to allocate the OLR, then wrap past > >> the end of the ring if necessary, then wait for space if necessary. > >> But subsequently intel_ring_begin() was refactored, in > >> > >>> 304d695 drm/i915: Flush outstanding requests before allocating new seqno > >> > >> to ensure that pending work that might need to be flushed used the old > >> and not the newly-allocated request. This changed the sequence to wrap > >> and/or wait, then allocate, although the comment still said > >>/* Preallocate the olr before touching the ring */ > >> which was no longer true as intel_wrap_ring_buffer() touches the ring. > >> > >> However, with the introduction of dynamic pinning, in > >> > >>> 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand > >> > >> came the possibility that the ringbuffer might not be pinned to the GTT > >> or mapped into CPU address space when intel_ring_begin() is called. It > >> gets pinned when the request is allocated, so it's now important that > >> this comes *before* anything that can write into the ringbuffer, in this > >> case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer > >> happens not to be mapped, and (b) tail happens to be sufficiently close > >> to the end of the ring to trigger wrapping. > > > > On the other hand, the request allocation can itself write into the > > ring. This is not the right fix, that is the elimination of olr itself > > and passing the request into intel_ring_begin. That way we can explicit > > in our ordering into ring access. > > -Chris > > AFAICS, request allocation can write into the ring only if it actually > has to flush some *pre-existing* OLR. [Aside: it can actually trigger > writing into a completely different ringbuffer, but not the one we're > handling here!] The worst-case sequence is: You forget that ultimately (or rather should have been in the design for execlists once the shortcomings of the ad hoc method were apparent) equest allocation will also be responsible for context management (since they are one and the same). > It only works because i915_gem_request_alloc() allocates the request > early but doesn't store it in the OLR until the end. > > OTOH I agree that the long-term answer is the elimination of the OLR; > this is really something of a stopgap until John H's Anti-OLR patchset > is merged. See my patches a year ago for a more complete cleanup. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rework order of operations in {__intel, logical}_ring_prepare()
On 12/06/15 19:05, Chris Wilson wrote: > On Fri, Jun 12, 2015 at 06:09:08PM +0100, Dave Gordon wrote: >> The original idea of preallocating the OLR was implemented in >> >>> 9d773091 drm/i915: Preallocate next seqno before touching the ring >> >> and the sequence of operations was to allocate the OLR, then wrap past >> the end of the ring if necessary, then wait for space if necessary. >> But subsequently intel_ring_begin() was refactored, in >> >>> 304d695 drm/i915: Flush outstanding requests before allocating new seqno >> >> to ensure that pending work that might need to be flushed used the old >> and not the newly-allocated request. This changed the sequence to wrap >> and/or wait, then allocate, although the comment still said >> /* Preallocate the olr before touching the ring */ >> which was no longer true as intel_wrap_ring_buffer() touches the ring. >> >> However, with the introduction of dynamic pinning, in >> >>> 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand >> >> came the possibility that the ringbuffer might not be pinned to the GTT >> or mapped into CPU address space when intel_ring_begin() is called. It >> gets pinned when the request is allocated, so it's now important that >> this comes *before* anything that can write into the ringbuffer, in this >> case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer >> happens not to be mapped, and (b) tail happens to be sufficiently close >> to the end of the ring to trigger wrapping. > > On the other hand, the request allocation can itself write into the > ring. This is not the right fix, that is the elimination of olr itself > and passing the request into intel_ring_begin. That way we can explicit > in our ordering into ring access. > -Chris AFAICS, request allocation can write into the ring only if it actually has to flush some *pre-existing* OLR. [Aside: it can actually trigger writing into a completely different ringbuffer, but not the one we're handling here!] The worst-case sequence is: i915_gem_request_alloc finds there's no OLR i915_gem_get_seqno finds the seqno is 0 i915_gem_init_seqno for_eash_ring do ... intel_ring_idle but no OLR, so OK It only works because i915_gem_request_alloc() allocates the request early but doesn't store it in the OLR until the end. OTOH I agree that the long-term answer is the elimination of the OLR; this is really something of a stopgap until John H's Anti-OLR patchset is merged. Although, the simplification of the wait-wrap/wait-space sequence is probably worthwhile in its own right, so if Anti-OLR gets merged first we can put the rest of the changes on top of that. It's only code inside the "if(!OLR)" section that would need to be removed. .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: use effective_size for ringbuffer calculations
On Fri, Jun 12, 2015 at 06:09:07PM +0100, Dave Gordon wrote: > When calculating the available space in a ringbuffer, we should > use the effective_size rather than the true size of the ring. > > v2: rebase to latest drm-intel-nightly > v3: rebase to latest drm-intel-nightly > > Signed-off-by: Dave Gordon > --- > drivers/gpu/drm/i915/intel_lrc.c|5 +++-- > drivers/gpu/drm/i915/intel_ringbuffer.c |9 ++--- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 9b74ffa..454e836 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -699,7 +699,7 @@ static int logical_ring_wait_for_space(struct > intel_ringbuffer *ringbuf, > > /* Would completion of this request free enough space? */ > space = __intel_ring_space(request->postfix, ringbuf->tail, > -ringbuf->size); > +ringbuf->effective_size); > if (space >= bytes) > break; > } > @@ -711,7 +711,8 @@ static int logical_ring_wait_for_space(struct > intel_ringbuffer *ringbuf, > if (ret) > return ret; > > - ringbuf->space = space; > + /* Update ring space after wait+retire */ > + intel_ring_update_space(ringbuf); Does the function not do what it says on the tin? At least make it seem like you are explaining your reasoning, not documenting the following function. /* * Having waited for the request, query the HEAD of most recent retired * request and use that for our space calcuations. */ However, that makes an incorrect assumption about the waiter. Given that the current code is written such that ringbuf->last_retired_head = request->postfix and that space is identical to the repeated calculation, what is your intention exactly? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Rework order of operations in {__intel, logical}_ring_prepare()
On Fri, Jun 12, 2015 at 06:09:08PM +0100, Dave Gordon wrote: > The original idea of preallocating the OLR was implemented in > > > 9d773091 drm/i915: Preallocate next seqno before touching the ring > > and the sequence of operations was to allocate the OLR, then wrap past > the end of the ring if necessary, then wait for space if necessary. > But subsequently intel_ring_begin() was refactored, in > > > 304d695 drm/i915: Flush outstanding requests before allocating new seqno > > to ensure that pending work that might need to be flushed used the old > and not the newly-allocated request. This changed the sequence to wrap > and/or wait, then allocate, although the comment still said > /* Preallocate the olr before touching the ring */ > which was no longer true as intel_wrap_ring_buffer() touches the ring. > > However, with the introduction of dynamic pinning, in > > > 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand > > came the possibility that the ringbuffer might not be pinned to the GTT > or mapped into CPU address space when intel_ring_begin() is called. It > gets pinned when the request is allocated, so it's now important that > this comes *before* anything that can write into the ringbuffer, in this > case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer > happens not to be mapped, and (b) tail happens to be sufficiently close > to the end of the ring to trigger wrapping. On the other hand, the request allocation can itself write into the ring. This is not the right fix, that is the elimination of olr itself and passing the request into intel_ring_begin. That way we can explicit in our ordering into ring access. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915/skl: Retrieve the Rpe value from Pcode
From: Akash Goel
Read the efficient frequency (aka RPe) value through the the mailbox
command (0x1A) from the pcode, as done on Haswell and Broadwell.
The turbo minimum frequency softlimit is not revised as per the
efficient frequency value.
v2: Replaced the conditional expression operator with 'if' statement (Tom)
v3: Corrected the derivation of efficient frequency & shifted the
GEN9_FREQ_SCALER multiplications downwards (Ville)
Issue: VIZ-5143
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/intel_pm.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d091fec..ff72374 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4303,18 +4303,11 @@ static void gen6_init_rps_frequencies(struct drm_device
*dev)
dev_priv->rps.rp0_freq = (rp_state_cap >> 0) & 0xff;
dev_priv->rps.rp1_freq = (rp_state_cap >> 8) & 0xff;
dev_priv->rps.min_freq = (rp_state_cap >> 16) & 0xff;
- if (IS_SKYLAKE(dev)) {
- /* Store the frequency values in 16.66 MHZ units, which is
- the natural hardware unit for SKL */
- dev_priv->rps.rp0_freq *= GEN9_FREQ_SCALER;
- dev_priv->rps.rp1_freq *= GEN9_FREQ_SCALER;
- dev_priv->rps.min_freq *= GEN9_FREQ_SCALER;
- }
/* hw_max = RP0 until we check for overclocking */
dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
ret = sandybridge_pcode_read(dev_priv,
HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
&ddcc_status);
@@ -4326,6 +4319,16 @@ static void gen6_init_rps_frequencies(struct drm_device
*dev)
dev_priv->rps.max_freq);
}
+ if (IS_SKYLAKE(dev)) {
+ /* Store the frequency values in 16.66 MHZ units, which is
+ the natural hardware unit for SKL */
+ dev_priv->rps.rp0_freq *= GEN9_FREQ_SCALER;
+ dev_priv->rps.rp1_freq *= GEN9_FREQ_SCALER;
+ dev_priv->rps.min_freq *= GEN9_FREQ_SCALER;
+ dev_priv->rps.max_freq *= GEN9_FREQ_SCALER;
+ dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
+ }
+
dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
/* Preserve min/max settings in case of re-init */
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] FBC trivial patches, V2
On Fri, Jun 12, 2015 at 02:36:17PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > Thanks for the V1 reviews and comments! > > Paulo Zanoni (4): > drm/i915: print FBC compression status on debugfs > drm/i915: add FBC_ROTATION to enum no_fbc_reason > drm/i915: unify no_fbc_reason message printing > drm/i915: don't set the FBC plane select bits on HSW+ Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] FBC trivial patches, V2
From: Paulo Zanoni Thanks for the V1 reviews and comments! Paulo Zanoni (4): drm/i915: print FBC compression status on debugfs drm/i915: add FBC_ROTATION to enum no_fbc_reason drm/i915: unify no_fbc_reason message printing drm/i915: don't set the FBC plane select bits on HSW+ drivers/gpu/drm/i915/i915_debugfs.c | 51 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 3 ++ drivers/gpu/drm/i915/intel_drv.h| 1 + drivers/gpu/drm/i915/intel_fbc.c| 75 - 5 files changed, 64 insertions(+), 67 deletions(-) -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: add FBC_ROTATION to enum no_fbc_reason
From: Paulo Zanoni
Because we're currently using FBC_UNSUPPORTED_MODE for two different
cases.
This commit will also allow us to write the next one without hiding
information from the user.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_fbc.c| 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index b3f42ec..f039b18 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1632,6 +1632,9 @@ static int i915_fbc_status(struct seq_file *m, void
*unused)
case FBC_CHIP_DEFAULT:
seq_puts(m, "disabled per chip default");
break;
+ case FBC_ROTATION:
+ seq_puts(m, "rotation not supported");
+ break;
default:
seq_puts(m, "unknown reason");
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 611fbd8..f9ff452 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -926,6 +926,7 @@ struct i915_fbc {
FBC_MULTIPLE_PIPES, /* more than one pipe active */
FBC_MODULE_PARAM,
FBC_CHIP_DEFAULT, /* disabled by default on this chip */
+ FBC_ROTATION, /* rotation is not supported */
} no_fbc_reason;
};
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 6abb834..43704a4 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -587,7 +587,7 @@ void intel_fbc_update(struct drm_device *dev)
}
if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
crtc->primary->state->rotation != BIT(DRM_ROTATE_0)) {
- if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+ if (set_no_fbc_reason(dev_priv, FBC_ROTATION))
DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
goto out_disable;
}
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+
From: Paulo Zanoni This commit is just to make the intentions explicit: on HSW+ these bits are MBZ, but since we only support plane A and the macro evaluates to zero when plane A is the parameter, we're not fixing any bug. v2: - Remove useless extra blank like (Chris). - Init dpfc_ctl in another place (Chris). Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 1ff288c..50ed333 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -262,7 +262,10 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) dev_priv->fbc.enabled = true; - dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane); + dpfc_ctl = 0; + if (IS_IVYBRIDGE(dev)) + dpfc_ctl |= IVB_DPFC_CTL_PLANE(intel_crtc->plane); + if (drm_format_plane_cpp(fb->pixel_format, 0) == 2) dev_priv->fbc.threshold++; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: unify no_fbc_reason message printing
From: Paulo Zanoni
This commit has two main advantages: simplify intel_fbc_update()
and deduplicate the strings.
v2:
- Rebase due to changes on P1.
- set_no_fbc_reason() can now return void (Chris).
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_debugfs.c | 49 +++---
drivers/gpu/drm/i915/intel_drv.h| 1 +
drivers/gpu/drm/i915/intel_fbc.c| 70 -
3 files changed, 51 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index f039b18..ee48b95 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void
*unused)
intel_runtime_pm_get(dev_priv);
- if (intel_fbc_enabled(dev)) {
+ if (intel_fbc_enabled(dev))
seq_puts(m, "FBC enabled\n");
- } else {
- seq_puts(m, "FBC disabled: ");
- switch (dev_priv->fbc.no_fbc_reason) {
- case FBC_OK:
- seq_puts(m, "FBC actived, but currently disabled in
hardware");
- break;
- case FBC_UNSUPPORTED:
- seq_puts(m, "unsupported by this chipset");
- break;
- case FBC_NO_OUTPUT:
- seq_puts(m, "no outputs");
- break;
- case FBC_STOLEN_TOO_SMALL:
- seq_puts(m, "not enough stolen memory");
- break;
- case FBC_UNSUPPORTED_MODE:
- seq_puts(m, "mode not supported");
- break;
- case FBC_MODE_TOO_LARGE:
- seq_puts(m, "mode too large");
- break;
- case FBC_BAD_PLANE:
- seq_puts(m, "FBC unsupported on plane");
- break;
- case FBC_NOT_TILED:
- seq_puts(m, "scanout buffer not tiled");
- break;
- case FBC_MULTIPLE_PIPES:
- seq_puts(m, "multiple pipes are enabled");
- break;
- case FBC_MODULE_PARAM:
- seq_puts(m, "disabled per module param (default off)");
- break;
- case FBC_CHIP_DEFAULT:
- seq_puts(m, "disabled per chip default");
- break;
- case FBC_ROTATION:
- seq_puts(m, "rotation not supported");
- break;
- default:
- seq_puts(m, "unknown reason");
- }
- seq_putc(m, '\n');
- }
+ else
+ seq_printf(m, "FBC disabled: %s\n",
+ intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
if (INTEL_INFO(dev_priv)->gen >= 7)
seq_printf(m, "Compressing: %s\n",
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b28029a..77f24e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1255,6 +1255,7 @@ void intel_fbc_invalidate(struct drm_i915_private
*dev_priv,
enum fb_op_origin origin);
void intel_fbc_flush(struct drm_i915_private *dev_priv,
unsigned int frontbuffer_bits);
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
/* intel_hdmi.c */
void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 43704a4..1ff288c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -432,14 +432,47 @@ void intel_fbc_disable(struct drm_device *dev)
dev_priv->fbc.crtc = NULL;
}
-static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
+const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
+{
+ switch (reason) {
+ case FBC_OK:
+ return "FBC enabled but currently disabled in hardware";
+ case FBC_UNSUPPORTED:
+ return "unsupported by this chipset";
+ case FBC_NO_OUTPUT:
+ return "no output";
+ case FBC_STOLEN_TOO_SMALL:
+ return "not enough stolen memory";
+ case FBC_UNSUPPORTED_MODE:
+ return "mode incompatible with compression";
+ case FBC_MODE_TOO_LARGE:
+ return "mode too large for compression";
+ case FBC_BAD_PLANE:
+ return "FBC unsupported on plane";
+ case FBC_NOT_TILED:
+ return "framebuffer not tiled or fenced";
+ case FBC_MULTIPLE_PIPES:
+ return "more than one pipe active";
+ case FBC_MODULE_PARAM:
+ return "disabled per module param";
+ case FBC_CHIP_DEFAULT:
+
[Intel-gfx] [PATCH 1/4] drm/i915: print FBC compression status on debugfs
From: Paulo Zanoni
We already had a few bugs in the past where FBC was compressing
nothing when it was enabled, which makes the feature quite useless.
Add this information to debugfs so the test suites can check for
regressions in this piece of the code.
Our igt/tests/kms_frontbuffer_tracking already has support for this
message.
v2: - Remove pointless VLV check (Ville).
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 +
drivers/gpu/drm/i915/i915_reg.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 92cf273..b3f42ec 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1638,6 +1638,11 @@ static int i915_fbc_status(struct seq_file *m, void
*unused)
seq_putc(m, '\n');
}
+ if (INTEL_INFO(dev_priv)->gen >= 7)
+ seq_printf(m, "Compressing: %s\n",
+ yesno(I915_READ(FBC_STATUS2) &
+FBC_COMPRESSION_MASK));
+
intel_runtime_pm_put(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 40a3a64..0c0b12a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1951,6 +1951,9 @@ enum skl_disp_power_wells {
#define FBC_FENCE_OFF 0x03218 /* BSpec typo has 321Bh */
#define FBC_TAG0x03300
+#define FBC_STATUS20x43214
+#define FBC_COMPRESSION_MASK 0x7ff
+
#define FBC_LL_SIZE(1536)
/* Framebuffer compression for GM45+ */
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 04/10] drm: Add Gamma correction structure
Hi Kausal Malladi,
On 5 June 2015 at 13:00, Jindal, Sonika wrote:
> On 6/4/2015 7:12 PM, Kausal Malladi wrote:
>>
>> From: Kausal Malladi
>>
...
>> v2: Addressing Daniel Stone's comment, added a variable sized array to
>> carry Gamma correction values as blob property.
>>
>> Signed-off-by: Shashank Sharma
>> Signed-off-by: Kausal Malladi
>> ---
>> include/drm/drm_crtc.h | 3 +++
>> include/uapi/drm/drm.h | 10 ++
>> 2 files changed, 13 insertions(+)
>>
...
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 3801584..fc2661c 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -829,6 +829,16 @@ struct drm_event_vblank {
>> __u32 reserved;
>> };
>>
>> +/* Color Management structure for Gamma */
>> +struct drm_gamma {
>> + __u32 flags;
>> + __u32 gamma_level;
>> + __u32 gamma_precision;
>> + __u32 num_samples;
>> + __u32 reserved;
>> + __u16 values[0];
Silly question:
Why use zero sized array ? Afaik it's a construct not covered in
C90/C99, which makes sizeof(struct drm_gamma) act funny. There seems
to be no other instance of a zero-sized array in drm uapi, plus based
of Daniel Vettel's "Botching up IOCTLS" I think that using it here
might be a bad idea.
The commit message mentions that Daniel Stone suggested it, but that
email never made it to the dri-devel mailiing list (and many other
emails, as mentioned previously) :'-(
Thanks
Emil
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: use effective_size for ringbuffer calculations
When calculating the available space in a ringbuffer, we should
use the effective_size rather than the true size of the ring.
v2: rebase to latest drm-intel-nightly
v3: rebase to latest drm-intel-nightly
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c|5 +++--
drivers/gpu/drm/i915/intel_ringbuffer.c |9 ++---
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9b74ffa..454e836 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -699,7 +699,7 @@ static int logical_ring_wait_for_space(struct
intel_ringbuffer *ringbuf,
/* Would completion of this request free enough space? */
space = __intel_ring_space(request->postfix, ringbuf->tail,
- ringbuf->size);
+ ringbuf->effective_size);
if (space >= bytes)
break;
}
@@ -711,7 +711,8 @@ static int logical_ring_wait_for_space(struct
intel_ringbuffer *ringbuf,
if (ret)
return ret;
- ringbuf->space = space;
+ /* Update ring space after wait+retire */
+ intel_ring_update_space(ringbuf);
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b70d25b..a3406b2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -66,7 +66,8 @@ void intel_ring_update_space(struct intel_ringbuffer *ringbuf)
}
ringbuf->space = __intel_ring_space(ringbuf->head & HEAD_ADDR,
- ringbuf->tail, ringbuf->size);
+ ringbuf->tail,
+ ringbuf->effective_size);
}
int intel_ring_space(struct intel_ringbuffer *ringbuf)
@@ -2117,8 +2118,9 @@ static int ring_wait_for_space(struct intel_engine_cs
*ring, int n)
return 0;
list_for_each_entry(request, &ring->request_list, list) {
+ /* Would completion of this request free enough space? */
space = __intel_ring_space(request->postfix, ringbuf->tail,
- ringbuf->size);
+ ringbuf->effective_size);
if (space >= n)
break;
}
@@ -2130,7 +2132,8 @@ static int ring_wait_for_space(struct intel_engine_cs
*ring, int n)
if (ret)
return ret;
- ringbuf->space = space;
+ /* Update ring space after wait+retire */
+ intel_ring_update_space(ringbuf);
return 0;
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Rework order of operations in {__intel, logical}_ring_prepare()
The original idea of preallocating the OLR was implemented in
> 9d773091 drm/i915: Preallocate next seqno before touching the ring
and the sequence of operations was to allocate the OLR, then wrap past
the end of the ring if necessary, then wait for space if necessary.
But subsequently intel_ring_begin() was refactored, in
> 304d695 drm/i915: Flush outstanding requests before allocating new seqno
to ensure that pending work that might need to be flushed used the old
and not the newly-allocated request. This changed the sequence to wrap
and/or wait, then allocate, although the comment still said
/* Preallocate the olr before touching the ring */
which was no longer true as intel_wrap_ring_buffer() touches the ring.
However, with the introduction of dynamic pinning, in
> 7ba717c drm/i915/bdw: Pin the ringbuffer backing object to GGTT on-demand
came the possibility that the ringbuffer might not be pinned to the GTT
or mapped into CPU address space when intel_ring_begin() is called. It
gets pinned when the request is allocated, so it's now important that
this comes *before* anything that can write into the ringbuffer, in this
case intel_wrap_ring_buffer(), as this will fault if (a) the ringbuffer
happens not to be mapped, and (b) tail happens to be sufficiently close
to the end of the ring to trigger wrapping.
So the correct order is neither the original allocate-wait-pad-wait, nor
the subsequent wait-pad-wait-allocate, but wait-allocate-pad, avoiding
both the problems described in the two commits mentioned above.
As a bonus, we eliminate the special case where a single ring_begin()
might end up waiting twice (once to be able to wrap, and then again
if that still hadn't actually freed enough space for the request). We
just precalculate the total amount of space we'll need *including* any
for padding the end of the ring and wait for that much in one go :)
In the time since this code was written, it has all been cloned from
the original ringbuffer model to become the execbuffer code, in
> 82e104c drm/i915/bdw: New logical ring submission mechanism
So now we have to fix it in both paths ...
Signed-off-by: Dave Gordon
---
drivers/gpu/drm/i915/intel_lrc.c| 64 +++
drivers/gpu/drm/i915/intel_ringbuffer.c | 63 +++---
2 files changed, 64 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 454e836..3ef5fb6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -740,39 +740,22 @@ intel_logical_ring_advance_and_submit(struct
intel_ringbuffer *ringbuf,
execlists_context_queue(ring, ctx, ringbuf->tail, request);
}
-static int logical_ring_wrap_buffer(struct intel_ringbuffer *ringbuf,
- struct intel_context *ctx)
-{
- uint32_t __iomem *virt;
- int rem = ringbuf->size - ringbuf->tail;
-
- if (ringbuf->space < rem) {
- int ret = logical_ring_wait_for_space(ringbuf, ctx, rem);
-
- if (ret)
- return ret;
- }
-
- virt = ringbuf->virtual_start + ringbuf->tail;
- rem /= 4;
- while (rem--)
- iowrite32(MI_NOOP, virt++);
-
- ringbuf->tail = 0;
- intel_ring_update_space(ringbuf);
-
- return 0;
-}
-
static int logical_ring_prepare(struct intel_ringbuffer *ringbuf,
struct intel_context *ctx, int bytes)
{
+ int fill = 0;
int ret;
+ /*
+* If the request will not fit between 'tail' and the effective
+* size of the ringbuffer, then we need to pad the end of the
+* ringbuffer with NOOPs, then start the request at the top of
+* the ring. This increases the total size that we need to check
+* for by however much is left at the end of the ring ...
+*/
if (unlikely(ringbuf->tail + bytes > ringbuf->effective_size)) {
- ret = logical_ring_wrap_buffer(ringbuf, ctx);
- if (unlikely(ret))
- return ret;
+ fill = ringbuf->size - ringbuf->tail;
+ bytes += fill;
}
if (unlikely(ringbuf->space < bytes)) {
@@ -781,6 +764,28 @@ static int logical_ring_prepare(struct intel_ringbuffer
*ringbuf,
return ret;
}
+ /* Ensure we have a request before touching the ring */
+ if (!ringbuf->ring->outstanding_lazy_request) {
+ ret = i915_gem_request_alloc(ringbuf->ring, ctx);
+ if (ret)
+ return ret;
+ }
+
+ if (unlikely(fill)) {
+ uint32_t __iomem *virt = ringbuf->virtual_start + ringbuf->tail;
+
+ /* tail should not have moved */
+ if (WARN_ON(fill != ringbuf->size - ringbuf->tail))
+ fill = ringbuf->size - ringbuf->tail;
+
+ do
+
[Intel-gfx] [PATCH v2] Resolve issues with ringbuffer space management
Updates and supersedes the referenced patch,
"Reinstate order of operations in {intel,logical}_ring_begin()"
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 6/6] drm/i915/gen8: Add WaRsRestoreWithPerCtxtBb workaround
On 12/06/15 12:58, Siluvery, Arun wrote: > On 09/06/2015 19:43, Dave Gordon wrote: >> On 05/06/15 14:57, Arun Siluvery wrote: >>> In Per context w/a batch buffer, >>> WaRsRestoreWithPerCtxtBb >>> >>> v2: This patches modifies definitions of MI_LOAD_REGISTER_MEM and >>> MI_LOAD_REGISTER_REG; Add GEN8 specific defines for these instructions >>> so as to not break any future users of existing definitions (Michel) >>> >>> Signed-off-by: Rafael Barbalho >>> Signed-off-by: Arun Siluvery >>> --- >>> drivers/gpu/drm/i915/i915_reg.h | 26 ++ >>> drivers/gpu/drm/i915/intel_lrc.c | 59 >>> >>> 2 files changed, 85 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index 33b0ff1..6928162 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >> [snip] >>> #define MI_LOAD_REGISTER_MEMMI_INSTR(0x29, 0) >>> #define MI_LOAD_REGISTER_REGMI_INSTR(0x2A, 0) >>> +#define MI_LOAD_REGISTER_MEM_GEN8 MI_INSTR(0x29, 2) >>> +#define MI_LRM_USE_GLOBAL_GTT (1<<22) >>> +#define MI_LRM_ASYNC_MODE_ENABLE (1<<21) >>> +#define MI_LOAD_REGISTER_REG_GEN8 MI_INSTR(0x2A, 1) >> >> Isn't the original definition of MI_LOAD_REGISTER_REG wrong anyway? It's >> a two-operand instruction, each of which is a one-word MMIO register >> address, hence always 3 words total. The length bias is 2, so the >> so-called 'flags' field must be 1. The original definition (where the >> second argument of the MI_INSTR macro is 0) shouldn't work. >> >> So just correct the original definition of MI_LOAD_REGISTER_REG; this >> isn't something that's actually changed on GEN8. >> > I did notice that the original instructions are odd but thought I might > be wrong hence I created new ones to not disturb the original ones. > ok I will just correct original one and reuse it. > >> While we're mentioning it, I think the above MI_LOAD_REGISTER_MEM is >> wrong too. The length should be 1 pre-GEN8, and 2 in GEN8+. >> > ok. >>> #define MI_RS_STORE_DATA_IMMMI_INSTR(0x2B, 0) >>> #define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0) >>> #define MI_STORE_URB_MEMMI_INSTR(0x2D, 0) >> >> And these are wrong too! In fact all of these instructions have been >> added under a comment which says "Commands used only by the command >> parser". Looks like they were added as placeholders without the proper >> length fields, and then people have started using them as though they >> were complete definitions :( >> >> Time update them all, perhaps ... > these are not related to this patch, so it can be taken up as a > different patch. As a minimum, you should move these updated #defines out of the section under the comment "Commands used only by the command parser" and put them in the appropriate place in the regular list of MI_ commnds, preferably in numerical order. Then the ones that are genuinely only used by the command parser could be left for another patch ... >> [snip] >> >>> +/* >>> + * BSpec says MI_LOAD_REGISTER_MEM, MI_LOAD_REGISTER_REG and >>> + * MI_BATCH_BUFFER_END instructions in this sequence need to be >>> + * in the same cacheline. >>> + */ >>> +while (((unsigned long) (cmd + index) % CACHELINE_BYTES) != 0) >>> +cmd[index++] = MI_NOOP; >>> + >>> +cmd[index++] = MI_LOAD_REGISTER_MEM_GEN8 | >>> +MI_LRM_USE_GLOBAL_GTT | >>> +MI_LRM_ASYNC_MODE_ENABLE; >>> +cmd[index++] = INSTPM; >>> +cmd[index++] = scratch_addr; >>> +cmd[index++] = 0; >>> + >>> +/* >>> + * BSpec says there should not be any commands programmed >>> + * between MI_LOAD_REGISTER_REG and MI_BATCH_BUFFER_END so >>> + * do not add any new commands >>> + */ >>> +cmd[index++] = MI_LOAD_REGISTER_REG_GEN8; >>> +cmd[index++] = GEN8_RS_PREEMPT_STATUS; >>> +cmd[index++] = GEN8_RS_PREEMPT_STATUS; >>> + >>> /* padding */ >>> while (index < end) >>> cmd[index++] = MI_NOOP; >>> >> >> Where's the MI_BATCH_BUFFER_END referred to in the comment? > > MI_BATCH_BUFFER_END is just below while loop, it is in patch [v3 1/6]. > Since the diff context is only few lines it didn't showup in the diff. The second comment above says "no commands between LOAD_REG_REG and BB_END", so the point of my comment was that the BB_END is *NOT* immediately after the LOAD_REG_REG -- there are a bunch of no-ops there! And therefore also, these instructions do *not* all end up in the same cacheline, thus contradicting the first comment above. Padding *after* a BB_END would be redundant. .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Bump CHV PFI credits to 63 when cdclk>=czclk
On 05/26/2015 10:22 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
Switch from using 31 PFI credits to 63 PFI credits when cdclk>=czclk on
CHV. The spec lists both 31 and 63 as "suggested" values, but based on
feedback from hardware folks we should actually be using 63. Originally
I picked the 31 basically by flipping a coin.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 067b1de..44b9c54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5986,7 +5986,7 @@ static void vlv_program_pfi_credits(struct
drm_i915_private *dev_priv)
if (DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 1000) >=
dev_priv->rps.cz_freq) {
/* CHV suggested value is 31 or 63 */
if (IS_CHERRYVIEW(dev_priv))
- credits = PFI_CREDIT_31;
+ credits = PFI_CREDIT_63;
else
credits = PFI_CREDIT(15);
} else {
Although not part of this review the else clause is setting PFI_CREDIT
to 15 when the BPSEC states that the default of 8 should be used when
cdclk/czclk < 1. According to the original patch, 15 is the optimal
value as stated by another driver team.
Reviewed-by: Clint Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Fix resume from suspend on IBM X30
Hi folks,
the attached patch fixes the resume from suspend on the IBM X30 (Bug
49838) by keeping a backup of the IVCH registers during initialization
and by replaying those values when re-setting a mode. The patch has been
successfully tested on an IBM X30 and on an IBM R31 laptop, both of
which make use of the same intel iVCH DVO.
Please let me know whether this patch is acceptable.
Thanks and have a nice weekend,
Thomas
>From 457d52863d4dfb9d68091fca5716560dd4eb4441 Mon Sep 17 00:00:00 2001
From: Thomas Richter
Date: Sat, 30 May 2015 20:25:53 +0200
Subject: [PATCH 1/1] Fix resume from suspend on IBM X30
This patch fixes the resume from suspend-to-ram on the IBM X30
laptop. The problem is caused by the Bios missing to re-initialize
the iVCH registers, especially the PLL registers.
This patch records the iVCH registers during initialization, and
re-installs this register set when resuming.
Signed-off-by: Thomas Richter
---
drivers/gpu/drm/i915/dvo_ivch.c | 63 +++
1 file changed, 57 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c
index 89b08a8..732ce87 100644
--- a/drivers/gpu/drm/i915/dvo_ivch.c
+++ b/drivers/gpu/drm/i915/dvo_ivch.c
@@ -22,6 +22,7 @@
*
* Authors:
*Eric Anholt
+ *Thomas Richter
*
* Minor modifications (Dithering enable):
*Thomas Richter
@@ -90,7 +91,7 @@
/*
* LCD Vertical Display Size
*/
-#define VR21 0x20
+#define VR21 0x21
/*
* Panel power down status
@@ -155,16 +156,33 @@
# define VR8F_POWER_MASK (0x3c)
# define VR8F_POWER_POS (2)
+/* Some Bios implementations do not restore the DVO state upon
+ * resume from standby. Thus, this driver has to handle it
+ * instead. The following list contains all registers that
+ * require saving.
+ */
+static const uint16_t backup_addresses[] = {
+ 0x11, 0x12,
+ 0x18, 0x19, 0x1a, 0x1f,
+ 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
+ 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37,
+ 0x8e, 0x8f,
+ 0x10 /* this must come last */
+};
+
struct ivch_priv {
bool quiet;
uint16_t width, height;
+
+ /* Register backup */
+
+ uint16_t reg_backup[ARRAY_SIZE(backup_addresses)];
};
static void ivch_dump_regs(struct intel_dvo_device *dvo);
-
/**
* Reads a register on the ivch.
*
@@ -246,6 +264,7 @@ static bool ivch_init(struct intel_dvo_device *dvo,
{
struct ivch_priv *priv;
uint16_t temp;
+ int i;
priv = kzalloc(sizeof(struct ivch_priv), GFP_KERNEL);
if (priv == NULL)
@@ -273,6 +292,14 @@ static bool ivch_init(struct intel_dvo_device *dvo,
ivch_read(dvo, VR20, &priv->width);
ivch_read(dvo, VR21, &priv->height);
+ /* Make a backup of the registers to be able to restore them
+ * upon suspend.
+ */
+ for (i = 0; i < ARRAY_SIZE(backup_addresses); i++)
+ ivch_read(dvo, backup_addresses[i], priv->reg_backup + i);
+
+ ivch_dump_regs(dvo);
+
return true;
out:
@@ -294,12 +321,31 @@ static enum drm_mode_status ivch_mode_valid(struct intel_dvo_device *dvo,
return MODE_OK;
}
+/* Restore the DVO registers after a resume
+ * from RAM. Registers have been saved during
+ * the initialization.
+ */
+static void ivch_reset(struct intel_dvo_device *dvo)
+{
+ struct ivch_priv *priv = dvo->dev_priv;
+ int i;
+
+ DRM_DEBUG_KMS("Resetting the IVCH registers\n");
+
+ ivch_write(dvo, VR10, 0x);
+
+ for (i = 0; i < ARRAY_SIZE(backup_addresses); i++)
+ ivch_write(dvo, backup_addresses[i], priv->reg_backup[i]);
+}
+
/** Sets the power state of the panel connected to the ivch */
static void ivch_dpms(struct intel_dvo_device *dvo, bool enable)
{
int i;
uint16_t vr01, vr30, backlight;
+ ivch_reset(dvo);
+
/* Set the new power state of the panel. */
if (!ivch_read(dvo, VR01, &vr01))
return;
@@ -308,6 +354,7 @@ static void ivch_dpms(struct intel_dvo_device *dvo, bool enable)
backlight = 1;
else
backlight = 0;
+
ivch_write(dvo, VR80, backlight);
if (enable)
@@ -334,6 +381,8 @@ static bool ivch_get_hw_state(struct intel_dvo_device *dvo)
{
uint16_t vr01;
+ ivch_reset(dvo);
+
/* Set the new power state of the panel. */
if (!ivch_read(dvo, VR01, &vr01))
return false;
@@ -348,11 +397,15 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
{
+ struct ivch_priv *priv = dvo->dev_priv;
uint16_t vr40 = 0;
uint16_t vr01 = 0;
uint16_t vr10;
- ivch_read(dvo, VR10, &vr10);
+ ivch_reset(dvo);
+
+ vr10 = priv->reg_backup[ARRAY_SIZE(backup_addresses) - 1];
+
/* Enable dithering for 18 bpp pipelines */
vr10 &= VR10_INTERFACE_DEPTH_MASK;
if (vr10 == VR10_INTERFACE_2X18 || vr10 == VR10_INTERFACE_1X18)
@@ -366,7 +419,7 @@ static void ivch_mode_set(struct intel_dvo_device *dvo,
uint16_t x_ratio, y_ratio;
vr01 |= VR01_PANEL_FIT_ENABLE;
- vr40 |= VR40_CLOCK_GATING_ENABLE | VR40_ENHANCED_PANEL_FITTING;
+ vr40 |= VR40_CLOCK_GATING_ENABLE;
x_ratio = (((mode->hdispla
Re: [Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
Op 12-06-15 om 13:43 schreef Ville Syrjälä:
> On Fri, Jun 12, 2015 at 02:27:54PM +0300, Ville Syrjälä wrote:
>> On Fri, Jun 12, 2015 at 12:31:37PM +0200, Maarten Lankhorst wrote:
>>> Op 12-06-15 om 12:16 schreef Ville Syrjälä:
On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
> Use a full atomic call instead. intel_crtc_page_flip will still
> have to live until async updates are allowed.
>
> This doesn't seem to be a regression from the convert to atomic,
> part 3 patch. During GPU reset it fixes the following warning:
>
> [ cut here ]
> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
> drm_mode_page_flip_ioctl+0x27b/0x360()
> Modules linked in: i915
> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
> 81c90866 8800d87c3ca8 817f7d87 8001
> 8800d87c3ce8 81084955 8800
> 8800d87c3dc0 8800d93d1208 8800b7d1f3e0
> Call Trace:
> [] dump_stack+0x4f/0x7b
> [] warn_slowpath_common+0x85/0xc0
> [] warn_slowpath_null+0x15/0x20
> [] drm_mode_page_flip_ioctl+0x27b/0x360
> [] drm_ioctl+0x1a0/0x6a0
> [] ? get_parent_ip+0x11/0x50
> [] ? avc_has_perm+0x20/0x280
> [] ? get_parent_ip+0x11/0x50
> [] do_vfs_ioctl+0x2f8/0x530
> [] ? expand_files+0x261/0x270
> [] ? selinux_file_ioctl+0x56/0x100
> [] SyS_ioctl+0x81/0xa0
> [] system_call_fastpath+0x12/0x6f
> ---[ end trace 9ce834560085bd64 ]---
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 7abaffeda7ce..cdf6549c8e74 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11618,8 +11618,35 @@ free_work:
> kfree(work);
>
> if (ret == -EIO) {
> + struct drm_atomic_state *state;
> + struct drm_plane_state *plane_state;
> +
> out_hang:
> - ret = intel_plane_restore(primary);
So what exactly is wrong with intel_plane_restore() (ie.
drm_plane_helper_update())? Shouldn't you fix that instead of spreading
the uglyness here?
>>> intel_plane_restore uses the transitional helpers. This is currently used
>>> for
>>> setting color key, enabling sprite planes after a modeset and this function.
>>> - Color key will be made atomic by making ckey part of the plane state.
>> Well what function do you plan to call there? The same should work here
>> no?
> To elaborate a bit more. My main gripe here is all this boilerplate we
> need. I hope we're planning to abstract that away a bit so we don't have
> to keep repeating it everywhere.
Eventually color key should probably become a property, but how does this look?
Adding color key with this function should be easy.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7abaffeda7ce..cda85eca7174 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11431,6 +11431,53 @@ void intel_check_page_flip(struct drm_device *dev, int
pipe)
spin_unlock(&dev->event_lock);
}
+static int intel_update_plane_state(struct drm_plane *plane,
+ struct drm_modeset_acquire_ctx *acquire_ctx,
+ int (*update_fn)(struct drm_plane_state *,
void *data),
+ void *data)
+{
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+ int ret;
+
+ state = drm_atomic_state_alloc(plane->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = acquire_ctx;
+
+retry:
+ plane_state = drm_atomic_get_plane_state(state, plane);
+ ret = PTR_ERR_OR_ZERO(plane_state);
+ if (!ret)
+ ret = update_fn(plane_state, data);
+
+ if (!ret)
+ ret = drm_atomic_commit(state);
+
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(state->acquire_ctx);
+ drm_atomic_state_clear(state);
+ goto retry;
+ }
+
+ if (ret)
+ drm_atomic_state_free(state);
+
+ return ret;
+}
+
+static int
+intel_crtc_page_flip_update_fb(struct drm_plane_state *plane_state, void *data)
+{
+ drm_atomic_set_fb_for_plane(plane_state, data);
+
+ if (WARN_ON(!plane_state->crtc))
+ return -EINVAL;
+
+ return 0;
+}
+
static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
Re: [Intel-gfx] [PATCH 0/4] Fix more fallout from reverting atomic hw readout.
Hey,
Op 12-06-15 om 14:45 schreef Jani Nikula:
> On Fri, 12 Jun 2015, Maarten Lankhorst
> wrote:
>> Commit f662af8c5c1619 reverts
>> "drm/i915: Read hw state into an atomic state struct, v2."
>> but it doesn't take into account other changes that were done in that time.
>> Before I continue on the atomic fixes I want to fix the fallout first,
>> and some of the reasons I identified that could cause a failure for atomic
>> modeset.
>>
>> When that's fixed I'll look at committing the atomic hw readout patch
>> again, since it will be needed for converting to full atomic.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929
> To recap, the atomic conversion part 2 series [1] broke things
> [2][3][4], and the quick fix reverts [5] broke other things [6], and now
> we have these patches to fix that.
>
> I was resolved to either merge these fixes or purge the whole thing this
> afternoon. On the one hand people have based their work on this and it
> clearly now works with the test coverage we've had, on the other hand
> I'm now less confident with the series overall, and Ville and Maarten
> keep on debating about it.
>
> I've ended up chickening out of this and compromising. I've moved
> ("demoted") Maarten's atomic series part 2 from drm-intel-next-queued to
> a new topical branch topic/atomic-conversion, and added these four
> patches on top. The topical branch merges to
> drm-intel-nightly. Basically nightly now has these patches on top, but
> the source of the commits is different.
We were discussing the intel_plane_restore patch, but that was to fix a hard
hang on my broadwell machine, that occurs when it terminally wedges.
I picked the patch from my tree because I already had it and the warning made
it looks like it would fix it. This was already broken before part 2 was
applied.
It can safely be dropped without affecting anyone affected by the hangs, the
other 3 patches should still be applied to the topic branch.
> Frankly what I'm doing is deferring this to Daniel who'll return next
> week. He'll take over handling drm-intel-next-queue on his return
> anyway, so given he'll have to deal with the rest of the fallout I think
> it's only fair he can decide what to do. I would have dropped the whole
> series and started over if I were in charge next week, but this will
> probably cause less of a hickup right now.
Sane decision, especially if he's back next monday. :-)
>
> BR,
> Jani.
>
>
> [1]
> http://mid.gmane.org/1433155811-9671-1-git-send-email-maarten.lankho...@linux.intel.com
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=90868
> [3] https://bugs.freedesktop.org/show_bug.cgi?id=90861
> [4] https://bugs.freedesktop.org/show_bug.cgi?id=90874
> [5]
> http://mid.gmane.org/1433924660-2228-1-git-send-email-maarten.lankho...@linux.intel.com
> [6] https://bugs.freedesktop.org/show_bug.cgi?id=90929
>
>
>> Maarten Lankhorst (4):
>> drm/i915: Do not use atomic modesets in hw readout.
>> drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
>> drm/i915: Set hwmode during readout.
>> drm/i915: Only enable cursor if it can be enabled.
>>
>> drivers/gpu/drm/i915/intel_display.c | 108
>> ---
>> 1 file changed, 63 insertions(+), 45 deletions(-)
>>
>> --
>> 2.1.0
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] Fix more fallout from reverting atomic hw readout.
On Fri, 12 Jun 2015, Maarten Lankhorst
wrote:
> Commit f662af8c5c1619 reverts
> "drm/i915: Read hw state into an atomic state struct, v2."
> but it doesn't take into account other changes that were done in that time.
> Before I continue on the atomic fixes I want to fix the fallout first,
> and some of the reasons I identified that could cause a failure for atomic
> modeset.
>
> When that's fixed I'll look at committing the atomic hw readout patch
> again, since it will be needed for converting to full atomic.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929
To recap, the atomic conversion part 2 series [1] broke things
[2][3][4], and the quick fix reverts [5] broke other things [6], and now
we have these patches to fix that.
I was resolved to either merge these fixes or purge the whole thing this
afternoon. On the one hand people have based their work on this and it
clearly now works with the test coverage we've had, on the other hand
I'm now less confident with the series overall, and Ville and Maarten
keep on debating about it.
I've ended up chickening out of this and compromising. I've moved
("demoted") Maarten's atomic series part 2 from drm-intel-next-queued to
a new topical branch topic/atomic-conversion, and added these four
patches on top. The topical branch merges to
drm-intel-nightly. Basically nightly now has these patches on top, but
the source of the commits is different.
Frankly what I'm doing is deferring this to Daniel who'll return next
week. He'll take over handling drm-intel-next-queue on his return
anyway, so given he'll have to deal with the rest of the fallout I think
it's only fair he can decide what to do. I would have dropped the whole
series and started over if I were in charge next week, but this will
probably cause less of a hickup right now.
BR,
Jani.
[1]
http://mid.gmane.org/1433155811-9671-1-git-send-email-maarten.lankho...@linux.intel.com
[2] https://bugs.freedesktop.org/show_bug.cgi?id=90868
[3] https://bugs.freedesktop.org/show_bug.cgi?id=90861
[4] https://bugs.freedesktop.org/show_bug.cgi?id=90874
[5]
http://mid.gmane.org/1433924660-2228-1-git-send-email-maarten.lankho...@linux.intel.com
[6] https://bugs.freedesktop.org/show_bug.cgi?id=90929
>
> Maarten Lankhorst (4):
> drm/i915: Do not use atomic modesets in hw readout.
> drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
> drm/i915: Set hwmode during readout.
> drm/i915: Only enable cursor if it can be enabled.
>
> drivers/gpu/drm/i915/intel_display.c | 108
> ---
> 1 file changed, 63 insertions(+), 45 deletions(-)
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: unify no_fbc_reason message printing
On Thu, Jun 11, 2015 at 04:02:26PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> This commit has two main advantages: simplify intel_fbc_update()
> and deduplicate the strings.
>
> Signed-off-by: Paulo Zanoni
I had some things around that topic as well in feb. May be of interest:
http://lists.freedesktop.org/archives/intel-gfx/2015-February/060850.html
--
Damien
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 49 +++
> drivers/gpu/drm/i915/intel_drv.h| 1 +
> drivers/gpu/drm/i915/intel_fbc.c| 66
> +
> 3 files changed, 50 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 405022b..eaa567c 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1594,52 +1594,11 @@ static int i915_fbc_status(struct seq_file *m, void
> *unused)
>
> intel_runtime_pm_get(dev_priv);
>
> - if (intel_fbc_enabled(dev)) {
> + if (intel_fbc_enabled(dev))
> seq_puts(m, "FBC enabled\n");
> - } else {
> - seq_puts(m, "FBC disabled: ");
> - switch (dev_priv->fbc.no_fbc_reason) {
> - case FBC_OK:
> - seq_puts(m, "FBC actived, but currently disabled in
> hardware");
> - break;
> - case FBC_UNSUPPORTED:
> - seq_puts(m, "unsupported by this chipset");
> - break;
> - case FBC_NO_OUTPUT:
> - seq_puts(m, "no outputs");
> - break;
> - case FBC_STOLEN_TOO_SMALL:
> - seq_puts(m, "not enough stolen memory");
> - break;
> - case FBC_UNSUPPORTED_MODE:
> - seq_puts(m, "mode not supported");
> - break;
> - case FBC_MODE_TOO_LARGE:
> - seq_puts(m, "mode too large");
> - break;
> - case FBC_BAD_PLANE:
> - seq_puts(m, "FBC unsupported on plane");
> - break;
> - case FBC_NOT_TILED:
> - seq_puts(m, "scanout buffer not tiled");
> - break;
> - case FBC_MULTIPLE_PIPES:
> - seq_puts(m, "multiple pipes are enabled");
> - break;
> - case FBC_MODULE_PARAM:
> - seq_puts(m, "disabled per module param (default off)");
> - break;
> - case FBC_CHIP_DEFAULT:
> - seq_puts(m, "disabled per chip default");
> - break;
> - case FBC_ROTATION:
> - seq_puts(m, "rotation not supported");
> - break;
> - default:
> - seq_puts(m, "unknown reason");
> - }
> - seq_putc(m, '\n');
> - }
> + else
> + seq_printf(m, "FBC disabled: %s\n",
> + intel_no_fbc_reason_str(dev_priv->fbc.no_fbc_reason));
>
> if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
> seq_printf(m, "Compressing: %s\n",
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index b28029a..77f24e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1255,6 +1255,7 @@ void intel_fbc_invalidate(struct drm_i915_private
> *dev_priv,
> enum fb_op_origin origin);
> void intel_fbc_flush(struct drm_i915_private *dev_priv,
>unsigned int frontbuffer_bits);
> +const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
>
> /* intel_hdmi.c */
> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> b/drivers/gpu/drm/i915/intel_fbc.c
> index 43704a4..9b300bd 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -432,6 +432,39 @@ void intel_fbc_disable(struct drm_device *dev)
> dev_priv->fbc.crtc = NULL;
> }
>
> +const char *intel_no_fbc_reason_str(enum no_fbc_reason reason)
> +{
> + switch (reason) {
> + case FBC_OK:
> + return "FBC enabled but currently disabled in hardware";
> + case FBC_UNSUPPORTED:
> + return "unsupported by this chipset";
> + case FBC_NO_OUTPUT:
> + return "no output";
> + case FBC_STOLEN_TOO_SMALL:
> + return "not enough stolen memory";
> + case FBC_UNSUPPORTED_MODE:
> + return "mode incompatible with compression";
> + case FBC_MODE_TOO_LARGE:
> + return "mode too large for compression";
> + case FBC_BAD_PLANE:
> + return "FBC unsupported on plane";
> + case FBC_NOT_TILED:
> + return "framebuffer not tiled or fenced";
> + ca
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Do not use atomic modesets in hw readout.
On Fri, Jun 12, 2015 at 11:15:39AM +0200, Maarten Lankhorst wrote:
> This should fix fallout caused by making intel_crtc_control
> and update_dpms atomic, which became a problem after reverting the
> atomic hw readout patch.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929
> Reported-by: Ville Syrjälä
> Signed-off-by: Maarten Lankhorst
So far my 830 and gen4 seem happy with this.
Tested-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 75
> +++-
> 1 file changed, 32 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 5cc2263db199..7abaffeda7ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6187,31 +6187,35 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
> mutex_unlock(&dev->struct_mutex);
> }
>
> +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
> +{
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> + enum intel_display_power_domain domain;
> + unsigned long domains;
> +
> + if (!intel_crtc->active)
> + return;
> +
> + intel_crtc_disable_planes(crtc);
> + dev_priv->display.crtc_disable(crtc);
> +
> + domains = intel_crtc->enabled_power_domains;
> + for_each_power_domain(domain, domains)
> + intel_display_power_put(dev_priv, domain);
> + intel_crtc->enabled_power_domains = 0;
> +}
> +
> /*
> * turn all crtc's off, but do not adjust state
> * This has to be paired with a call to intel_modeset_setup_hw_state.
> */
> void intel_display_suspend(struct drm_device *dev)
> {
> - struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_crtc *crtc;
>
> - for_each_crtc(dev, crtc) {
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - enum intel_display_power_domain domain;
> - unsigned long domains;
> -
> - if (!intel_crtc->active)
> - continue;
> -
> - intel_crtc_disable_planes(crtc);
> - dev_priv->display.crtc_disable(crtc);
> -
> - domains = intel_crtc->enabled_power_domains;
> - for_each_power_domain(domain, domains)
> - intel_display_power_put(dev_priv, domain);
> - intel_crtc->enabled_power_domains = 0;
> - }
> + for_each_crtc(dev, crtc)
> + intel_crtc_disable_noatomic(crtc);
> }
>
> /* Master function to enable/disable CRTC and corresponding power wells */
> @@ -15120,7 +15124,9 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> {
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_encoder *encoder;
> u32 reg;
> + bool enable;
>
> /* Clear any frame start delays used for debugging left by the BIOS */
> reg = PIPECONF(crtc->config->cpu_transcoder);
> @@ -15137,7 +15143,6 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
>* disable the crtc (and hence change the state) if it is wrong. Note
>* that gen4+ has a fixed plane -> pipe mapping. */
> if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
> - struct intel_connector *connector;
> bool plane;
>
> DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
> @@ -15149,29 +15154,8 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> plane = crtc->plane;
> to_intel_plane_state(crtc->base.primary->state)->visible = true;
> crtc->plane = !plane;
> - intel_crtc_control(&crtc->base, false);
> + intel_crtc_disable_noatomic(&crtc->base);
> crtc->plane = plane;
> -
> - /* ... and break all links. */
> - for_each_intel_connector(dev, connector) {
> - if (connector->encoder->base.crtc != &crtc->base)
> - continue;
> -
> - connector->base.dpms = DRM_MODE_DPMS_OFF;
> - connector->base.encoder = NULL;
> - }
> - /* multiple connectors may have the same encoder:
> - * handle them and break crtc link separately */
> - for_each_intel_connector(dev, connector)
> - if (connector->encoder->base.crtc == &crtc->base) {
> - connector->encoder->base.crtc = NULL;
> - connector->encoder->connectors_active = false;
> - }
> -
> - WARN_ON(crtc->active);
> - crtc->base.state->enable = false;
> - crtc->base.state->active = false;
> - crtc->base.enabled = false;
> }
>
> if (dev_priv->quirks & QUIRK_PI
Re: [Intel-gfx] [PATCH 1/4] drm/i915: print FBC compression status on debugfs
On Thu, Jun 11, 2015 at 04:02:24PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni
>
> We already had a few bugs in the past where FBC was compressing
> nothing when it was enabled, which makes the feature quite useless.
> Add this information to debugfs so the test suites can check for
> regressions in this piece of the code.
>
> Our igt/tests/kms_frontbuffer_tracking already has support for this
> message.
>
> Signed-off-by: Paulo Zanoni
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 5 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 92cf273..7358f6d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1638,6 +1638,11 @@ static int i915_fbc_status(struct seq_file *m, void
> *unused)
> seq_putc(m, '\n');
> }
>
> + if (INTEL_INFO(dev_priv)->gen >= 7 && !IS_VALLEYVIEW(dev_priv))
We already have HAS_FBC check in this function so the VLV check is
pointless.
> + seq_printf(m, "Compressing: %s\n",
> +yesno(I915_READ(FBC_STATUS2) &
> + FBC_COMPRESSION_MASK));
> +
> intel_runtime_pm_put(dev_priv);
>
> return 0;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 40a3a64..0c0b12a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1951,6 +1951,9 @@ enum skl_disp_power_wells {
> #define FBC_FENCE_OFF0x03218 /* BSpec typo has 321Bh */
> #define FBC_TAG 0x03300
>
> +#define FBC_STATUS2 0x43214
> +#define FBC_COMPRESSION_MASK0x7ff
> +
> #define FBC_LL_SIZE (1536)
>
> /* Framebuffer compression for GM45+ */
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 6/6] drm/i915/gen8: Add WaRsRestoreWithPerCtxtBb workaround
On 09/06/2015 19:43, Dave Gordon wrote: On 05/06/15 14:57, Arun Siluvery wrote: In Per context w/a batch buffer, WaRsRestoreWithPerCtxtBb v2: This patches modifies definitions of MI_LOAD_REGISTER_MEM and MI_LOAD_REGISTER_REG; Add GEN8 specific defines for these instructions so as to not break any future users of existing definitions (Michel) Signed-off-by: Rafael Barbalho Signed-off-by: Arun Siluvery --- drivers/gpu/drm/i915/i915_reg.h | 26 ++ drivers/gpu/drm/i915/intel_lrc.c | 59 2 files changed, 85 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 33b0ff1..6928162 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h [snip] #define MI_LOAD_REGISTER_MEMMI_INSTR(0x29, 0) #define MI_LOAD_REGISTER_REGMI_INSTR(0x2A, 0) +#define MI_LOAD_REGISTER_MEM_GEN8 MI_INSTR(0x29, 2) +#define MI_LRM_USE_GLOBAL_GTT (1<<22) +#define MI_LRM_ASYNC_MODE_ENABLE (1<<21) +#define MI_LOAD_REGISTER_REG_GEN8 MI_INSTR(0x2A, 1) Isn't the original definition of MI_LOAD_REGISTER_REG wrong anyway? It's a two-operand instruction, each of which is a one-word MMIO register address, hence always 3 words total. The length bias is 2, so the so-called 'flags' field must be 1. The original definition (where the second argument of the MI_INSTR macro is 0) shouldn't work. So just correct the original definition of MI_LOAD_REGISTER_REG; this isn't something that's actually changed on GEN8. I did notice that the original instructions are odd but thought I might be wrong hence I created new ones to not disturb the original ones. ok I will just correct original one and reuse it. While we're mentioning it, I think the above MI_LOAD_REGISTER_MEM is wrong too. The length should be 1 pre-GEN8, and 2 in GEN8+. ok. #define MI_RS_STORE_DATA_IMMMI_INSTR(0x2B, 0) #define MI_LOAD_URB_MEM MI_INSTR(0x2C, 0) #define MI_STORE_URB_MEMMI_INSTR(0x2D, 0) And these are wrong too! In fact all of these instructions have been added under a comment which says "Commands used only by the command parser". Looks like they were added as placeholders without the proper length fields, and then people have started using them as though they were complete definitions :( Time update them all, perhaps ... these are not related to this patch, so it can be taken up as a different patch. [snip] + /* +* BSpec says MI_LOAD_REGISTER_MEM, MI_LOAD_REGISTER_REG and +* MI_BATCH_BUFFER_END instructions in this sequence need to be +* in the same cacheline. +*/ + while (((unsigned long) (cmd + index) % CACHELINE_BYTES) != 0) + cmd[index++] = MI_NOOP; + + cmd[index++] = MI_LOAD_REGISTER_MEM_GEN8 | + MI_LRM_USE_GLOBAL_GTT | + MI_LRM_ASYNC_MODE_ENABLE; + cmd[index++] = INSTPM; + cmd[index++] = scratch_addr; + cmd[index++] = 0; + + /* +* BSpec says there should not be any commands programmed +* between MI_LOAD_REGISTER_REG and MI_BATCH_BUFFER_END so +* do not add any new commands +*/ + cmd[index++] = MI_LOAD_REGISTER_REG_GEN8; + cmd[index++] = GEN8_RS_PREEMPT_STATUS; + cmd[index++] = GEN8_RS_PREEMPT_STATUS; + /* padding */ while (index < end) cmd[index++] = MI_NOOP; Where's the MI_BATCH_BUFFER_END referred to in the comment? MI_BATCH_BUFFER_END is just below while loop, it is in patch [v3 1/6]. Since the diff context is only few lines it didn't showup in the diff. regards Arun .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/6] drm/i915/gen8: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround
On 05/06/2015 15:48, Ville Syrjälä wrote: On Fri, Jun 05, 2015 at 02:56:48PM +0100, Arun Siluvery wrote: In Indirect context w/a batch buffer, +WaFlushCoherentL3CacheLinesAtContextSwitch Signed-off-by: Rafael Barbalho Signed-off-by: Arun Siluvery --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 8 2 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 84af255..5203c79 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -426,6 +426,7 @@ #define PIPE_CONTROL_INDIRECT_STATE_DISABLE (1<<9) #define PIPE_CONTROL_NOTIFY (1<<8) #define PIPE_CONTROL_FLUSH_ENABLE (1<<7) /* gen7+ */ +#define PIPE_CONTROL_DC_FLUSH_ENABLE (1<<5) #define PIPE_CONTROL_VF_CACHE_INVALIDATE(1<<4) #define PIPE_CONTROL_CONST_CACHE_INVALIDATE (1<<3) #define PIPE_CONTROL_STATE_CACHE_INVALIDATE (1<<2) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a71eb81..9d8cf65c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1101,6 +1101,14 @@ static int gen8_init_indirectctx_bb(struct intel_engine_cs *ring) /* WaDisableCtxRestoreArbitration:bdw,chv */ cmd[index++] = MI_ARB_ON_OFF | MI_ARB_DISABLE; + /* WaFlushCoherentL3CacheLinesAtContextSwitch:bdw,chv */ + cmd[index++] = GFX_OP_PIPE_CONTROL(6); + cmd[index++] = PIPE_CONTROL_DC_FLUSH_ENABLE; + cmd[index++] = 0; + cmd[index++] = 0; + cmd[index++] = 0; + cmd[index++] = 0; + This looks incomplete. Seems like you should have LRIs around this guy to enable/disable the L3SQCREG4 coherent line flush bit. And chv shouldn't do coherent L3, so this might not be needed there. I checked with HW team and yes I need to add LRIs to enable/disable L3SQCREG4 coherent line flush bit. As you mentioned, it is not required for CHV. Also do we need a CS stall here too? "DC Flush Enable 5 Requires stall bit ([20] of DW) set for all GPGPU and Media Workloads." I didn't check the restrictions of this bit, will check again and correc this. regards Arun Supposedly we should add the DC flush to the normal ring flush hooks too. But that's a separate issue. /* padding */ while (index < end) cmd[index++] = MI_NOOP; -- 2.3.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.
On Fri, Jun 12, 2015 at 12:32:24PM +0200, Maarten Lankhorst wrote: > Op 12-06-15 om 12:26 schreef Ville Syrjälä: > > On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: > >> The cursor should only be enabled if it's visible. This fixes > >> igt/kms_cursor_crc, which may otherwise produce the following > >> warning: > >> > >> [ cut here ] > >> WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 > >> intel_crtc_update_cursor+0x14c/0x4d0 [i915]() > >> Missing switch case (0) in i9xx_update_cursor > >> Modules linked in: i915 > >> CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: GW > >> 4.1.0-rc7-patser+ #4079 > >> Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > >> c01aad10 8800b083faa8 817f7827 8001 > >> 8800b083faf8 8800b083fae8 81084955 8800b083fad8 > >> 8800c4931148 0120 8800c48b > >> Call Trace: > >> [] dump_stack+0x4f/0x7b > >> [] warn_slowpath_common+0x85/0xc0 > >> [] warn_slowpath_fmt+0x41/0x50 > >> [] intel_crtc_update_cursor+0x14c/0x4d0 [i915] > >> [] __intel_set_mode+0x6c4/0x750 [i915] > >> [] intel_crtc_set_config+0x473/0x5c0 [i915] > >> [] drm_mode_set_config_internal+0x69/0x120 > >> [] drm_mode_setcrtc+0x189/0x540 > >> [] drm_ioctl+0x1a0/0x6a0 > >> [] ? get_parent_ip+0x11/0x50 > >> [] do_vfs_ioctl+0x2f8/0x530 > >> [] ? trace_hardirqs_on+0xd/0x10 > >> [] ? selinux_file_ioctl+0x56/0x100 > >> [] SyS_ioctl+0x81/0xa0 > >> [] system_call_fastpath+0x12/0x6f > >> ---[ end trace abf0f71163290a96 ]--- > >> > >> Signed-off-by: Maarten Lankhorst > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> b/drivers/gpu/drm/i915/intel_display.c > >> index 14ccf49b9067..afe91a8f7e36 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc > >> *crtc) > >> > >>intel_enable_primary_hw_plane(crtc->primary, crtc); > >>intel_enable_sprite_planes(crtc); > >> - intel_crtc_update_cursor(crtc, true); > >> + if (to_intel_plane_state(crtc->cursor->state)->visible) > >> + intel_crtc_update_cursor(crtc, true); > > Can we actually trust it now? Last time I looked we couldn't, and > > Daniel didn't want my fixes to make it so. > > > > So if we can't trust 'visible' I suppose you should just do > > something a bit more ugly and look at 'crtc_w' or something. > > > We add all planes during a modeset, so state->visible should be sane. If we don't already have it, I'd like to see a test case that does: - set a big mode - set up all kinds of planes near the bottom right corner - set a small mode This should test that all active planes get clipped properly during the modeset. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
On Fri, Jun 12, 2015 at 02:27:54PM +0300, Ville Syrjälä wrote:
> On Fri, Jun 12, 2015 at 12:31:37PM +0200, Maarten Lankhorst wrote:
> > Op 12-06-15 om 12:16 schreef Ville Syrjälä:
> > > On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
> > >> Use a full atomic call instead. intel_crtc_page_flip will still
> > >> have to live until async updates are allowed.
> > >>
> > >> This doesn't seem to be a regression from the convert to atomic,
> > >> part 3 patch. During GPU reset it fixes the following warning:
> > >>
> > >> [ cut here ]
> > >> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
> > >> drm_mode_page_flip_ioctl+0x27b/0x360()
> > >> Modules linked in: i915
> > >> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
> > >> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355
> > >> 03/09/2015
> > >> 81c90866 8800d87c3ca8 817f7d87 8001
> > >> 8800d87c3ce8 81084955 8800
> > >> 8800d87c3dc0 8800d93d1208 8800b7d1f3e0
> > >> Call Trace:
> > >> [] dump_stack+0x4f/0x7b
> > >> [] warn_slowpath_common+0x85/0xc0
> > >> [] warn_slowpath_null+0x15/0x20
> > >> [] drm_mode_page_flip_ioctl+0x27b/0x360
> > >> [] drm_ioctl+0x1a0/0x6a0
> > >> [] ? get_parent_ip+0x11/0x50
> > >> [] ? avc_has_perm+0x20/0x280
> > >> [] ? get_parent_ip+0x11/0x50
> > >> [] do_vfs_ioctl+0x2f8/0x530
> > >> [] ? expand_files+0x261/0x270
> > >> [] ? selinux_file_ioctl+0x56/0x100
> > >> [] SyS_ioctl+0x81/0xa0
> > >> [] system_call_fastpath+0x12/0x6f
> > >> ---[ end trace 9ce834560085bd64 ]---
> > >>
> > >> Signed-off-by: Maarten Lankhorst
> > >> ---
> > >> drivers/gpu/drm/i915/intel_display.c | 29 -
> > >> 1 file changed, 28 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> > >> b/drivers/gpu/drm/i915/intel_display.c
> > >> index 7abaffeda7ce..cdf6549c8e74 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -11618,8 +11618,35 @@ free_work:
> > >> kfree(work);
> > >>
> > >> if (ret == -EIO) {
> > >> +struct drm_atomic_state *state;
> > >> +struct drm_plane_state *plane_state;
> > >> +
> > >> out_hang:
> > >> -ret = intel_plane_restore(primary);
> > > So what exactly is wrong with intel_plane_restore() (ie.
> > > drm_plane_helper_update())? Shouldn't you fix that instead of spreading
> > > the uglyness here?
> > >
> > intel_plane_restore uses the transitional helpers. This is currently used
> > for
> > setting color key, enabling sprite planes after a modeset and this function.
> > - Color key will be made atomic by making ckey part of the plane state.
>
> Well what function do you plan to call there? The same should work here
> no?
To elaborate a bit more. My main gripe here is all this boilerplate we
need. I hope we're planning to abstract that away a bit so we don't have
to keep repeating it everywhere.
So maybe it could look something like this:
ret = prep_plane_update()
set stuff in the state
ret = check_and_commit()
That way the EDEADLOCK mess etc. would be hidden inside the
prep part.
In the meantime I'd at least stuff this thing into separate functions so
that the ugly if (!ret) stuff would go away.
>
> > - Sprite plane updates after modeset will be made unnecessary by calling
> > drm_atomic_helper_commit_planes_on_crtc after .crtc_enable.
> > - This function needs to update plane->fb, and intel_plane_restore doesn't
> > provide that.
> >
> > That leaves only this function calling intel_plane_restore, if we can get
> > rid of it
> > there will be no need of transitional helper calls any more.
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Retrieve the Rpe value from Pcode
On Fri, 2015-06-12 at 13:32 +0300, Ville Syrjälä wrote:
> On Fri, Jun 12, 2015 at 03:01:08PM +0530, akash.g...@intel.com wrote:
> > From: Akash Goel
> >
> > Read the efficient frequency (aka RPe) value through the the mailbox
> > command (0x1A) from the pcode, as done on Haswell and Broadwell.
> > The turbo minimum frequency softlimit is not revised as per the
> > efficient frequency value.
> >
> > v2: Replaced the conditional expression operator with 'if' statement (Tom)
> >
> > Issue: VIZ-5143
> > Signed-off-by: Akash Goel
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index d091fec..21b22a7 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4314,16 +4314,20 @@ static void gen6_init_rps_frequencies(struct
> > drm_device *dev)
> > dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
> >
> > dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> > - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > + if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
> > ret = sandybridge_pcode_read(dev_priv,
> > HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> > &ddcc_status);
> > - if (0 == ret)
> > + if (0 == ret) {
> > dev_priv->rps.efficient_freq =
> > clamp_t(u8,
> > ((ddcc_status >> 8) & 0xff),
> > dev_priv->rps.min_freq,
> > dev_priv->rps.max_freq);
>
> That's wrong now since min/max_freq were already multiplied by
> GEN9_FREQ_SCALER.
Thanks for catching this issue. Sorry for the lapse.
> > +
> > + if (IS_SKYLAKE(dev))
> > + dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
> > + }
> > }
>
> I would suggest moving all the GEN9_FREQ_SCALER multiplications here.
>
Fine this will be cleaner.
Can I do this movement as a part of this patch only ?
Best regards
Akash
> >
> > dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> > --
> > 1.9.2
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4] FBC trivial patches
On Thu, Jun 11, 2015 at 04:02:23PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > Let's try to get the easy stuff merged while the rest is not ready > yet. > > This is a nice opportunity for you to easily increase your patch > review count! Sigh. I was until you said this. A couple of minor comments on 3 and 4. Otherwise ltgm. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
On Fri, Jun 12, 2015 at 12:31:37PM +0200, Maarten Lankhorst wrote:
> Op 12-06-15 om 12:16 schreef Ville Syrjälä:
> > On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
> >> Use a full atomic call instead. intel_crtc_page_flip will still
> >> have to live until async updates are allowed.
> >>
> >> This doesn't seem to be a regression from the convert to atomic,
> >> part 3 patch. During GPU reset it fixes the following warning:
> >>
> >> [ cut here ]
> >> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
> >> drm_mode_page_flip_ioctl+0x27b/0x360()
> >> Modules linked in: i915
> >> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
> >> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
> >> 81c90866 8800d87c3ca8 817f7d87 8001
> >> 8800d87c3ce8 81084955 8800
> >> 8800d87c3dc0 8800d93d1208 8800b7d1f3e0
> >> Call Trace:
> >> [] dump_stack+0x4f/0x7b
> >> [] warn_slowpath_common+0x85/0xc0
> >> [] warn_slowpath_null+0x15/0x20
> >> [] drm_mode_page_flip_ioctl+0x27b/0x360
> >> [] drm_ioctl+0x1a0/0x6a0
> >> [] ? get_parent_ip+0x11/0x50
> >> [] ? avc_has_perm+0x20/0x280
> >> [] ? get_parent_ip+0x11/0x50
> >> [] do_vfs_ioctl+0x2f8/0x530
> >> [] ? expand_files+0x261/0x270
> >> [] ? selinux_file_ioctl+0x56/0x100
> >> [] SyS_ioctl+0x81/0xa0
> >> [] system_call_fastpath+0x12/0x6f
> >> ---[ end trace 9ce834560085bd64 ]---
> >>
> >> Signed-off-by: Maarten Lankhorst
> >> ---
> >> drivers/gpu/drm/i915/intel_display.c | 29 -
> >> 1 file changed, 28 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index 7abaffeda7ce..cdf6549c8e74 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11618,8 +11618,35 @@ free_work:
> >>kfree(work);
> >>
> >>if (ret == -EIO) {
> >> + struct drm_atomic_state *state;
> >> + struct drm_plane_state *plane_state;
> >> +
> >> out_hang:
> >> - ret = intel_plane_restore(primary);
> > So what exactly is wrong with intel_plane_restore() (ie.
> > drm_plane_helper_update())? Shouldn't you fix that instead of spreading
> > the uglyness here?
> >
> intel_plane_restore uses the transitional helpers. This is currently used for
> setting color key, enabling sprite planes after a modeset and this function.
> - Color key will be made atomic by making ckey part of the plane state.
Well what function do you plan to call there? The same should work here
no?
> - Sprite plane updates after modeset will be made unnecessary by calling
> drm_atomic_helper_commit_planes_on_crtc after .crtc_enable.
> - This function needs to update plane->fb, and intel_plane_restore doesn't
> provide that.
>
> That leaves only this function calling intel_plane_restore, if we can get rid
> of it
> there will be no need of transitional helper calls any more.
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: unify no_fbc_reason message printing
On Thu, Jun 11, 2015 at 04:02:26PM -0300, Paulo Zanoni wrote:
> @@ -439,6 +472,8 @@ static bool set_no_fbc_reason(struct drm_i915_private
> *dev_priv,
> return false;
>
> dev_priv->fbc.no_fbc_reason = reason;
> + DRM_DEBUG_KMS("Disabling FBC: %s\n", intel_no_fbc_reason_str(reason));
> +
> return true;
The bool return is now unused.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: don't set the FBC plane select bits on HSW+
On Thu, Jun 11, 2015 at 04:02:27PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > > This commit is just to make the intentions explicit: on HSW+ these > bits are MBZ, but since we only support plane A and the macro > evaluates to zero when plane A is the parameter, we're not fixing any > bug. > > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_fbc.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index 9b300bd..8b980e5 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -258,11 +258,14 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) > struct drm_framebuffer *fb = crtc->primary->fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - u32 dpfc_ctl; > + u32 dpfc_ctl = 0; > > dev_priv->fbc.enabled = true; > > - dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane); > + An extra line of whitespace, just because. Minor bikeshed would be to dpfc_ctl = 0 here, so that the construction of dpfc_ctl is in a single logical block (admittedly in this case you have have to read back a few lines to find the initializer). > + if (IS_IVYBRIDGE(dev)) > + dpfc_ctl |= IVB_DPFC_CTL_PLANE(intel_crtc->plane); -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Actually respect DSPSURF alignment restrictions
On Thu, Jun 11, 2015 at 02:51:19PM +0100, Chris Wilson wrote:
> On Thu, Jun 11, 2015 at 04:31:14PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Currently intel_gen4_compute_page_offset() simply picks the closest
> > page boundary below the linear offset. That however may not be suitably
> > aligned to satisfy any hardware specific restrictions. So let's make
> > sure the page boundary we choose is properly aligned.
> >
> > Also to play it a bit safer lets split the remaining linear offset into
> > x and y values instead of just x. This should make no difference for
> > most platforms since we convert the x and y offsets back into a linear
> > offset before feeding them to the hardware. HSW+ are different however
> > and use x and y offsets even with linear buffers, so they might have
> > trouble if either the x or y get too big.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
>
> > @@ -2455,12 +2461,13 @@ unsigned long intel_gen4_compute_page_offset(int
> > *x, int *y,
> >
> > return tile_rows * pitch * 8 + tiles * 4096;
> > } else {
> > + unsigned int alignment = intel_linear_alignment(dev_priv) - 1;
> > unsigned int offset;
> >
> > offset = *y * pitch + *x * cpp;
> > - *y = 0;
> > - *x = (offset & 4095) / cpp;
> > - return offset & -4096;
> > + *y = (offset & alignment) / pitch;
> > + *x = ((offset & alignment) - *y * pitch) / cpp;
> > + return offset & ~alignment;
>
> Calculation looks solid. I presume we have a igt/kms test that combines
> linear/tiled, large surfaces and large offsets?
kms_plane has some kind of panning tests. Probably not as good as it
could/should be. I have a few custom tests I created to hunt for the
VLV/CHV bug, but those aren't really useable as regular igt tests as
is. Would take a bit of extra effort to turn them into such.
>
> Reviewed-by: Chris Wilson
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v5] libs/igt_core.c: Fix compile warnings in igt_core.c
Fixed variables incorrectly declared as int instead of size_t.
v2: Addressed comments from Tim Gore
v3: Removed 'unused parameter' changes
v4: Changed to size_t
v5: Moved declarations out of for loops
Signed-off-by: Derek Morton
---
lib/igt_core.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 8a1a249..eb0cb21 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -1104,7 +1104,9 @@ static pid_t helper_process_pids[] =
static void reset_helper_process_list(void)
{
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++)
helper_process_pids[i] = -1;
helper_process_count = 0;
}
@@ -1121,8 +1123,10 @@ static int __waitpid(pid_t pid)
static void fork_helper_exit_handler(int sig)
{
+ size_t i;
+
/* Inside a signal handler, play safe */
- for (int i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
+ for (i = 0; i < ARRAY_SIZE(helper_process_pids); i++) {
pid_t pid = helper_process_pids[i];
if (pid != -1) {
kill(pid, SIGTERM);
@@ -1376,10 +1380,10 @@ static void restore_sig_handler(int sig_num)
static void restore_all_sig_handler(void)
{
- int i;
+ size_t i;
for (i = 0; i < ARRAY_SIZE(orig_sig); i++)
- restore_sig_handler(i);
+ restore_sig_handler((int)i);
}
static void call_exit_handlers(int sig)
@@ -1419,7 +1423,7 @@ static bool crash_signal(int sig)
static void fatal_sig_handler(int sig)
{
- int i;
+ size_t i;
for (i = 0; i < ARRAY_SIZE(handled_signals); i++) {
if (handled_signals[i].number != sig)
@@ -1481,7 +1485,7 @@ static void fatal_sig_handler(int sig)
*/
void igt_install_exit_handler(igt_exit_handler_t fn)
{
- int i;
+ size_t i;
for (i = 0; i < exit_handler_count; i++)
if (exit_handler_fn[i] == fn)
@@ -1521,7 +1525,7 @@ err:
void igt_disable_exit_handler(void)
{
sigset_t set;
- int i;
+ size_t i;
if (exit_handler_disabled)
return;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Retrieve the Rpe value from Pcode
On Fri, Jun 12, 2015 at 03:01:08PM +0530, akash.g...@intel.com wrote:
> From: Akash Goel
>
> Read the efficient frequency (aka RPe) value through the the mailbox
> command (0x1A) from the pcode, as done on Haswell and Broadwell.
> The turbo minimum frequency softlimit is not revised as per the
> efficient frequency value.
>
> v2: Replaced the conditional expression operator with 'if' statement (Tom)
>
> Issue: VIZ-5143
> Signed-off-by: Akash Goel
> ---
> drivers/gpu/drm/i915/intel_pm.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d091fec..21b22a7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4314,16 +4314,20 @@ static void gen6_init_rps_frequencies(struct
> drm_device *dev)
> dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
>
> dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
> ret = sandybridge_pcode_read(dev_priv,
> HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> &ddcc_status);
> - if (0 == ret)
> + if (0 == ret) {
> dev_priv->rps.efficient_freq =
> clamp_t(u8,
> ((ddcc_status >> 8) & 0xff),
> dev_priv->rps.min_freq,
> dev_priv->rps.max_freq);
That's wrong now since min/max_freq were already multiplied by
GEN9_FREQ_SCALER.
> +
> + if (IS_SKYLAKE(dev))
> + dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
> + }
> }
I would suggest moving all the GEN9_FREQ_SCALER multiplications here.
>
> dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> --
> 1.9.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.
Op 12-06-15 om 12:26 schreef Ville Syrjälä: > On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: >> The cursor should only be enabled if it's visible. This fixes >> igt/kms_cursor_crc, which may otherwise produce the following >> warning: >> >> [ cut here ] >> WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 >> intel_crtc_update_cursor+0x14c/0x4d0 [i915]() >> Missing switch case (0) in i9xx_update_cursor >> Modules linked in: i915 >> CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: GW >> 4.1.0-rc7-patser+ #4079 >> Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 >> c01aad10 8800b083faa8 817f7827 8001 >> 8800b083faf8 8800b083fae8 81084955 8800b083fad8 >> 8800c4931148 0120 8800c48b >> Call Trace: >> [] dump_stack+0x4f/0x7b >> [] warn_slowpath_common+0x85/0xc0 >> [] warn_slowpath_fmt+0x41/0x50 >> [] intel_crtc_update_cursor+0x14c/0x4d0 [i915] >> [] __intel_set_mode+0x6c4/0x750 [i915] >> [] intel_crtc_set_config+0x473/0x5c0 [i915] >> [] drm_mode_set_config_internal+0x69/0x120 >> [] drm_mode_setcrtc+0x189/0x540 >> [] drm_ioctl+0x1a0/0x6a0 >> [] ? get_parent_ip+0x11/0x50 >> [] do_vfs_ioctl+0x2f8/0x530 >> [] ? trace_hardirqs_on+0xd/0x10 >> [] ? selinux_file_ioctl+0x56/0x100 >> [] SyS_ioctl+0x81/0xa0 >> [] system_call_fastpath+0x12/0x6f >> ---[ end trace abf0f71163290a96 ]--- >> >> Signed-off-by: Maarten Lankhorst >> --- >> drivers/gpu/drm/i915/intel_display.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 14ccf49b9067..afe91a8f7e36 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc >> *crtc) >> >> intel_enable_primary_hw_plane(crtc->primary, crtc); >> intel_enable_sprite_planes(crtc); >> -intel_crtc_update_cursor(crtc, true); >> +if (to_intel_plane_state(crtc->cursor->state)->visible) >> +intel_crtc_update_cursor(crtc, true); > Can we actually trust it now? Last time I looked we couldn't, and > Daniel didn't want my fixes to make it so. > > So if we can't trust 'visible' I suppose you should just do > something a bit more ugly and look at 'crtc_w' or something. > We add all planes during a modeset, so state->visible should be sane. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
Op 12-06-15 om 12:16 schreef Ville Syrjälä:
> On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
>> Use a full atomic call instead. intel_crtc_page_flip will still
>> have to live until async updates are allowed.
>>
>> This doesn't seem to be a regression from the convert to atomic,
>> part 3 patch. During GPU reset it fixes the following warning:
>>
>> [ cut here ]
>> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
>> drm_mode_page_flip_ioctl+0x27b/0x360()
>> Modules linked in: i915
>> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
>> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
>> 81c90866 8800d87c3ca8 817f7d87 8001
>> 8800d87c3ce8 81084955 8800
>> 8800d87c3dc0 8800d93d1208 8800b7d1f3e0
>> Call Trace:
>> [] dump_stack+0x4f/0x7b
>> [] warn_slowpath_common+0x85/0xc0
>> [] warn_slowpath_null+0x15/0x20
>> [] drm_mode_page_flip_ioctl+0x27b/0x360
>> [] drm_ioctl+0x1a0/0x6a0
>> [] ? get_parent_ip+0x11/0x50
>> [] ? avc_has_perm+0x20/0x280
>> [] ? get_parent_ip+0x11/0x50
>> [] do_vfs_ioctl+0x2f8/0x530
>> [] ? expand_files+0x261/0x270
>> [] ? selinux_file_ioctl+0x56/0x100
>> [] SyS_ioctl+0x81/0xa0
>> [] system_call_fastpath+0x12/0x6f
>> ---[ end trace 9ce834560085bd64 ]---
>>
>> Signed-off-by: Maarten Lankhorst
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 29 -
>> 1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 7abaffeda7ce..cdf6549c8e74 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11618,8 +11618,35 @@ free_work:
>> kfree(work);
>>
>> if (ret == -EIO) {
>> +struct drm_atomic_state *state;
>> +struct drm_plane_state *plane_state;
>> +
>> out_hang:
>> -ret = intel_plane_restore(primary);
> So what exactly is wrong with intel_plane_restore() (ie.
> drm_plane_helper_update())? Shouldn't you fix that instead of spreading
> the uglyness here?
>
intel_plane_restore uses the transitional helpers. This is currently used for
setting color key, enabling sprite planes after a modeset and this function.
- Color key will be made atomic by making ckey part of the plane state.
- Sprite plane updates after modeset will be made unnecessary by calling
drm_atomic_helper_commit_planes_on_crtc after .crtc_enable.
- This function needs to update plane->fb, and intel_plane_restore doesn't
provide that.
That leaves only this function calling intel_plane_restore, if we can get rid
of it
there will be no need of transitional helper calls any more.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.
On Fri, Jun 12, 2015 at 11:15:42AM +0200, Maarten Lankhorst wrote: > The cursor should only be enabled if it's visible. This fixes > igt/kms_cursor_crc, which may otherwise produce the following > warning: > > [ cut here ] > WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 > intel_crtc_update_cursor+0x14c/0x4d0 [i915]() > Missing switch case (0) in i9xx_update_cursor > Modules linked in: i915 > CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: GW > 4.1.0-rc7-patser+ #4079 > Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 > c01aad10 8800b083faa8 817f7827 8001 > 8800b083faf8 8800b083fae8 81084955 8800b083fad8 > 8800c4931148 0120 8800c48b > Call Trace: > [] dump_stack+0x4f/0x7b > [] warn_slowpath_common+0x85/0xc0 > [] warn_slowpath_fmt+0x41/0x50 > [] intel_crtc_update_cursor+0x14c/0x4d0 [i915] > [] __intel_set_mode+0x6c4/0x750 [i915] > [] intel_crtc_set_config+0x473/0x5c0 [i915] > [] drm_mode_set_config_internal+0x69/0x120 > [] drm_mode_setcrtc+0x189/0x540 > [] drm_ioctl+0x1a0/0x6a0 > [] ? get_parent_ip+0x11/0x50 > [] do_vfs_ioctl+0x2f8/0x530 > [] ? trace_hardirqs_on+0xd/0x10 > [] ? selinux_file_ioctl+0x56/0x100 > [] SyS_ioctl+0x81/0xa0 > [] system_call_fastpath+0x12/0x6f > ---[ end trace abf0f71163290a96 ]--- > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/intel_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 14ccf49b9067..afe91a8f7e36 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc > *crtc) > > intel_enable_primary_hw_plane(crtc->primary, crtc); > intel_enable_sprite_planes(crtc); > - intel_crtc_update_cursor(crtc, true); > + if (to_intel_plane_state(crtc->cursor->state)->visible) > + intel_crtc_update_cursor(crtc, true); Can we actually trust it now? Last time I looked we couldn't, and Daniel didn't want my fixes to make it so. So if we can't trust 'visible' I suppose you should just do something a bit more ugly and look at 'crtc_w' or something. > > intel_post_enable_primary(crtc); > > -- > 2.1.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/skl: Retrieve the Rpe value from Pcode
From: Akash Goel
Read the efficient frequency (aka RPe) value through the the mailbox
command (0x1A) from the pcode, as done on Haswell and Broadwell.
The turbo minimum frequency softlimit is not revised as per the
efficient frequency value.
v2: Replaced the conditional expression operator with 'if' statement (Tom)
Issue: VIZ-5143
Signed-off-by: Akash Goel
---
drivers/gpu/drm/i915/intel_pm.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d091fec..21b22a7 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4314,16 +4314,20 @@ static void gen6_init_rps_frequencies(struct drm_device
*dev)
dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev) || IS_SKYLAKE(dev)) {
ret = sandybridge_pcode_read(dev_priv,
HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
&ddcc_status);
- if (0 == ret)
+ if (0 == ret) {
dev_priv->rps.efficient_freq =
clamp_t(u8,
((ddcc_status >> 8) & 0xff),
dev_priv->rps.min_freq,
dev_priv->rps.max_freq);
+
+ if (IS_SKYLAKE(dev))
+ dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
+ }
}
dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
--
1.9.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Set hwmode during readout.
On Fri, Jun 12, 2015 at 11:15:41AM +0200, Maarten Lankhorst wrote: > This was introduced after converting hw readout to atomic, > so it should have been part of the revert too. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929 > Reported-by: Ville Syrjälä > Signed-off-by: Maarten Lankhorst Tested-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/intel_display.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index cdf6549c8e74..14ccf49b9067 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15357,6 +15357,7 @@ static void intel_modeset_readout_hw_state(struct > drm_device *dev) > crtc->base.state->enable = crtc->active; > crtc->base.state->active = crtc->active; > crtc->base.enabled = crtc->active; > + crtc->base.hwmode = crtc->config->base.adjusted_mode; > > plane_state = to_intel_plane_state(primary->state); > plane_state->visible = primary_get_hw_state(crtc); > -- > 2.1.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
On Fri, Jun 12, 2015 at 11:15:40AM +0200, Maarten Lankhorst wrote:
> Use a full atomic call instead. intel_crtc_page_flip will still
> have to live until async updates are allowed.
>
> This doesn't seem to be a regression from the convert to atomic,
> part 3 patch. During GPU reset it fixes the following warning:
>
> [ cut here ]
> WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
> drm_mode_page_flip_ioctl+0x27b/0x360()
> Modules linked in: i915
> CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
> 81c90866 8800d87c3ca8 817f7d87 8001
> 8800d87c3ce8 81084955 8800
> 8800d87c3dc0 8800d93d1208 8800b7d1f3e0
> Call Trace:
> [] dump_stack+0x4f/0x7b
> [] warn_slowpath_common+0x85/0xc0
> [] warn_slowpath_null+0x15/0x20
> [] drm_mode_page_flip_ioctl+0x27b/0x360
> [] drm_ioctl+0x1a0/0x6a0
> [] ? get_parent_ip+0x11/0x50
> [] ? avc_has_perm+0x20/0x280
> [] ? get_parent_ip+0x11/0x50
> [] do_vfs_ioctl+0x2f8/0x530
> [] ? expand_files+0x261/0x270
> [] ? selinux_file_ioctl+0x56/0x100
> [] SyS_ioctl+0x81/0xa0
> [] system_call_fastpath+0x12/0x6f
> ---[ end trace 9ce834560085bd64 ]---
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 29 -
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 7abaffeda7ce..cdf6549c8e74 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11618,8 +11618,35 @@ free_work:
> kfree(work);
>
> if (ret == -EIO) {
> + struct drm_atomic_state *state;
> + struct drm_plane_state *plane_state;
> +
> out_hang:
> - ret = intel_plane_restore(primary);
So what exactly is wrong with intel_plane_restore() (ie.
drm_plane_helper_update())? Shouldn't you fix that instead of spreading
the uglyness here?
> + state = drm_atomic_state_alloc(dev);
> + if (!state)
> + return -ENOMEM;
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> +retry:
> + plane_state = drm_atomic_get_plane_state(state, primary);
> + ret = PTR_ERR_OR_ZERO(plane_state);
> + if (!ret) {
> + drm_atomic_set_fb_for_plane(plane_state, fb);
> +
> + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> + if (!ret)
> + ret = drm_atomic_commit(state);
> + }
> +
> + if (ret == -EDEADLK) {
> + drm_modeset_backoff(state->acquire_ctx);
> + drm_atomic_state_clear(state);
> + goto retry;
> + }
> +
> + if (ret)
> + drm_atomic_state_free(state);
> +
> if (ret == 0 && event) {
> spin_lock_irq(&dev->event_lock);
> drm_send_vblank_event(dev, pipe, event);
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v4] lib/tests/igt_segfault Add unit test to test segfault handling
Unit test to check a segfaulting subtest is handled correctly.
v2: Added script to check subtest results
v3: Removed script. Updated test to use fork to monitor return status.
v4: Added igt_segfault to .gitignore
Signed-off-by: Derek Morton
---
lib/tests/.gitignore | 1 +
lib/tests/Makefile.sources | 1 +
lib/tests/igt_segfault.c | 139 +
3 files changed, 141 insertions(+)
create mode 100644 lib/tests/igt_segfault.c
diff --git a/lib/tests/.gitignore b/lib/tests/.gitignore
index a745a23..729568b 100644
--- a/lib/tests/.gitignore
+++ b/lib/tests/.gitignore
@@ -5,6 +5,7 @@ igt_list_only
igt_no_exit
igt_no_exit_list_only
igt_no_subtest
+igt_segfault
igt_simple_test_subtests
igt_simulation
igt_timeout
diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
index 10e0617..5fa0b31 100644
--- a/lib/tests/Makefile.sources
+++ b/lib/tests/Makefile.sources
@@ -8,6 +8,7 @@ check_PROGRAMS = \
igt_simple_test_subtests \
igt_timeout \
igt_invalid_subtest_name \
+ igt_segfault \
$(NULL)
check_SCRIPTS = \
diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c
new file mode 100644
index 000..b420b1a
--- /dev/null
+++ b/lib/tests/igt_segfault.c
@@ -0,0 +1,139 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Derek Morton
+ *
+ */
+
+/*
+ * Testcase: Test the framework catches a segfault and returns an error.
+ *
+ * 1. Test a crashing simple test is reported.
+ * 2. Test a crashing subtest is reported.
+ * 3. Test a crashing subtest following a passing subtest is reported.
+ * 4. Test a crashing subtest preceeding a passing subtest is reported.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "drmtest.h"
+#include "igt_core.h"
+
+/*
+ * We need to hide assert from the cocci igt test refactor spatch.
+ *
+ * IMPORTANT: Test infrastructure tests are the only valid places where using
+ * assert is allowed.
+ */
+#define internal_assert assert
+
+bool simple;
+bool runa;
+bool runc;
+char test[] = "test";
+char *argv_run[] = { test };
+
+static int do_fork(void)
+{
+ int pid, status;
+ int argc;
+ void (*crashme)(void) = NULL;
+
+ switch (pid = fork()) {
+ case -1:
+ internal_assert(0);
+ case 0:
+ if (simple) {
+ argc = 1;
+ igt_simple_init(argc, argv_run);
+ crashme();
+
+ igt_exit();
+ } else {
+
+ argc = 1;
+ igt_subtest_init(argc, argv_run);
+
+ if(runa)
+ igt_subtest("A")
+ ;
+
+ igt_subtest("B")
+ crashme();
+
+ if(runc)
+ igt_subtest("C")
+ ;
+
+ igt_exit();
+ }
+ default:
+ while (waitpid(pid, &status, 0) == -1 &&
+ errno == EINTR)
+ ;
+
+ if(WIFSIGNALED(status))
+ return WTERMSIG(status) + 128;
+
+ return WEXITSTATUS(status);
+ }
+}
+
+int main(int argc, char **argv)
+{
+ /* Test Crash in simple test is reported */
+ simple = true;
+ runa=false;
+ runc=false;
+ igt_info("Simple test.\n");
+ fflush(stdout);
+ internal_assert(do_fork() == SIGSEGV + 128);
+
+ /* Test crash in a single subtest is reported */
+ simple = false;
+ igt_info("Single subtest.\n");
+ fflush(stdout);
+ internal_assert(do_fork() == SIGSEGV + 128);
+
+ /* Test crash in a subtest following a pass is rep
Re: [Intel-gfx] [PATCH i-g-t v4] lib/tests/igt_segfault Add unit test to test segfault handling
This is the same as the previously submitted patch except the text encoding
should (hopefully) now be correct.
//Derek
>
>
>-Original Message-
>From: Morton, Derek J
>Sent: Friday, June 12, 2015 11:05 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Wood, Thomas; dan...@ffwll.ch; Morton, Derek J
>Subject: [PATCH i-g-t v4] lib/tests/igt_segfault Add unit test to test
>segfault handling
>
>Unit test to check a segfaulting subtest is handled correctly.
>
>v2: Added script to check subtest results
>v3: Removed script. Updated test to use fork to monitor return status.
>v4: Added igt_segfault to .gitignore
>
>Signed-off-by: Derek Morton
>---
> lib/tests/.gitignore | 1 +
> lib/tests/Makefile.sources | 1 +
> lib/tests/igt_segfault.c | 139 +
> 3 files changed, 141 insertions(+)
> create mode 100644 lib/tests/igt_segfault.c
>
>diff --git a/lib/tests/.gitignore b/lib/tests/.gitignore index
>a745a23..729568b 100644
>--- a/lib/tests/.gitignore
>+++ b/lib/tests/.gitignore
>@@ -5,6 +5,7 @@ igt_list_only
> igt_no_exit
> igt_no_exit_list_only
> igt_no_subtest
>+igt_segfault
> igt_simple_test_subtests
> igt_simulation
> igt_timeout
>diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources index
>10e0617..5fa0b31 100644
>--- a/lib/tests/Makefile.sources
>+++ b/lib/tests/Makefile.sources
>@@ -8,6 +8,7 @@ check_PROGRAMS = \
> igt_simple_test_subtests \
> igt_timeout \
> igt_invalid_subtest_name \
>+ igt_segfault \
> $(NULL)
>
> check_SCRIPTS = \
>diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c new file mode
>100644 index 000..b420b1a
>--- /dev/null
>+++ b/lib/tests/igt_segfault.c
>@@ -0,0 +1,139 @@
>+/*
>+ * Copyright © 2015 Intel Corporation
>+ *
>+ * Permission is hereby granted, free of charge, to any person
>+obtaining a
>+ * copy of this software and associated documentation files (the
>+"Software"),
>+ * to deal in the Software without restriction, including without
>+limitation
>+ * the rights to use, copy, modify, merge, publish, distribute,
>+sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom
>+the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice (including the
>+next
>+ * paragraph) shall be included in all copies or substantial portions
>+of the
>+ * Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>+EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>+MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>+SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>+OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>+ARISING
>+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>+DEALINGS
>+ * IN THE SOFTWARE.
>+ *
>+ * Authors:
>+ *Derek Morton
>+ *
>+ */
>+
>+/*
>+ * Testcase: Test the framework catches a segfault and returns an error.
>+ *
>+ * 1. Test a crashing simple test is reported.
>+ * 2. Test a crashing subtest is reported.
>+ * 3. Test a crashing subtest following a passing subtest is reported.
>+ * 4. Test a crashing subtest preceeding a passing subtest is reported.
>+ */
>+
>+#include
>+#include
>+#include
>+#include
>+#include
>+
>+#include "drmtest.h"
>+#include "igt_core.h"
>+
>+/*
>+ * We need to hide assert from the cocci igt test refactor spatch.
>+ *
>+ * IMPORTANT: Test infrastructure tests are the only valid places where
>+using
>+ * assert is allowed.
>+ */
>+#define internal_assert assert
>+
>+bool simple;
>+bool runa;
>+bool runc;
>+char test[] = "test";
>+char *argv_run[] = { test };
>+
>+static int do_fork(void)
>+{
>+ int pid, status;
>+ int argc;
>+ void (*crashme)(void) = NULL;
>+
>+ switch (pid = fork()) {
>+ case -1:
>+ internal_assert(0);
>+ case 0:
>+ if (simple) {
>+ argc = 1;
>+ igt_simple_init(argc, argv_run);
>+ crashme();
>+
>+ igt_exit();
>+ } else {
>+
>+ argc = 1;
>+ igt_subtest_init(argc, argv_run);
>+
>+ if(runa)
>+ igt_subtest("A")
>+ ;
>+
>+ igt_subtest("B")
>+ crashme();
>+
>+ if(runc)
>+ igt_subtest("C")
>+ ;
>+
>+ igt_exit();
>+ }
>+ default:
>+ while (waitpid(pid, &status, 0) == -1 &&
>+ errno == EINTR)
>+ ;
>+
>+ if(WIFSIGNALED(status))
>+ return WTERMSIG(status) + 128;
>+
>+ return WEXITST
[Intel-gfx] [PATCH v4] drm/i915/bxt: eDP Panel Power sequencing
Changes for BXT - added a IS_BROXTON check to use the macro related to PPS
registers for BXT.
BXT does not have PP_DIV register. Making changes to handle this.
Second set of PPS registers have been defined but will be used when VBT
provides a selection between the 2 sets of registers.
v2:
[Jani] Added 2nd set of PPS registers and the macro
Jani's review comments
- remove reference in i915_suspend.c
- Use BXT PP macro
Squashing all PPS related patches into one.
v3: Jani's review comments addressed
- Use pp_ctl instead of pp
- ironlake_get_pp_control() is not required for BXT
- correct the use of && in the print statement
- drop the shift in the print statement
v4: Jani's comments
- modify ironlake_get_pp_control() - dont set unlock key for bxt
Signed-off-by: Vandana Kannan
Signed-off-by: A.Sunil Kamath
Cc: Jani Nikula
---
drivers/gpu/drm/i915/i915_reg.h | 13 +++
drivers/gpu/drm/i915/intel_dp.c | 76 -
2 files changed, 72 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7213224..cc03b5b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6377,6 +6377,8 @@ enum skl_disp_power_wells {
#define PCH_PP_CONTROL 0xc7204
#define PANEL_UNLOCK_REGS (0xabcd << 16)
#define PANEL_UNLOCK_MASK (0x << 16)
+#define BXT_POWER_CYCLE_DELAY_MASK(0x1f0)
+#define BXT_POWER_CYCLE_DELAY_SHIFT 4
#define EDP_FORCE_VDD (1 << 3)
#define EDP_BLC_ENABLE(1 << 2)
#define PANEL_POWER_RESET (1 << 1)
@@ -6405,6 +6407,17 @@ enum skl_disp_power_wells {
#define PANEL_POWER_CYCLE_DELAY_MASK (0x1f)
#define PANEL_POWER_CYCLE_DELAY_SHIFT 0
+/* BXT PPS changes - 2nd set of PPS registers */
+#define _BXT_PP_STATUS20xc7300
+#define _BXT_PP_CONTROL2 0xc7304
+#define _BXT_PP_ON_DELAYS2 0xc7308
+#define _BXT_PP_OFF_DELAYS20xc730c
+
+#define BXT_PP_STATUS(n) ((!n) ? PCH_PP_STATUS : _BXT_PP_STATUS2)
+#define BXT_PP_CONTROL(n) ((!n) ? PCH_PP_CONTROL : _BXT_PP_CONTROL2)
+#define BXT_PP_ON_DELAYS(n)((!n) ? PCH_PP_ON_DELAYS : _BXT_PP_ON_DELAYS2)
+#define BXT_PP_OFF_DELAYS(n) ((!n) ? PCH_PP_OFF_DELAYS : _BXT_PP_OFF_DELAYS2)
+
#define PCH_DP_B 0xe4100
#define PCH_DPB_AUX_CH_CTL 0xe4110
#define PCH_DPB_AUX_CH_DATA1 0xe4114
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 15e8892..ad9cb0e9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -567,7 +567,9 @@ static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- if (HAS_PCH_SPLIT(dev))
+ if (IS_BROXTON(dev))
+ return BXT_PP_CONTROL(0);
+ else if (HAS_PCH_SPLIT(dev))
return PCH_PP_CONTROL;
else
return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
@@ -577,7 +579,9 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- if (HAS_PCH_SPLIT(dev))
+ if (IS_BROXTON(dev))
+ return BXT_PP_STATUS(0);
+ else if (HAS_PCH_SPLIT(dev))
return PCH_PP_STATUS;
else
return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
@@ -1701,8 +1705,10 @@ static u32 ironlake_get_pp_control(struct intel_dp
*intel_dp)
lockdep_assert_held(&dev_priv->pps_mutex);
control = I915_READ(_pp_ctrl_reg(intel_dp));
- control &= ~PANEL_UNLOCK_MASK;
- control |= PANEL_UNLOCK_REGS;
+ if (!IS_BROXTON(dev)) {
+ control &= ~PANEL_UNLOCK_MASK;
+ control |= PANEL_UNLOCK_REGS;
+ }
return control;
}
@@ -5091,8 +5097,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device
*dev,
struct drm_i915_private *dev_priv = dev->dev_private;
struct edp_power_seq cur, vbt, spec,
*final = &intel_dp->pps_delays;
- u32 pp_on, pp_off, pp_div, pp;
- int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
+ u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0;
+ int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg = 0;
lockdep_assert_held(&dev_priv->pps_mutex);
@@ -5100,7 +5106,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device
*dev,
if (final->t11_t12 != 0)
return;
- if (HAS_PCH_SPLIT(dev)) {
+ if (IS_BROXTON(dev)) {
+ /*
+* TODO: BXT has 2 sets of PPS registers.
+* Correct Register for Broxton need to be identified
+* using VBT. hardcoding for now
+*/
+ pp_ctrl_reg = BXT_PP_CONTROL(0);
+ pp_on_reg = BXT_PP_ON_DELAYS(0);
+ pp_off_reg = BXT_PP_OFF_DELAYS(0);
+ } else if (HAS_PCH_SP
[Intel-gfx] [PATCH] drm/atomic: pass old crtc state to atomic_begin/flush.
In intel it's useful to keep track of some state changes with old
crtc state vs new state, for example to disable initial planes or
when a modeset's prevented during fastboot.
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 6 --
drivers/gpu/drm/drm_atomic_helper.c| 8
drivers/gpu/drm/drm_plane_helper.c | 4 ++--
drivers/gpu/drm/i915/intel_display.c | 10 ++
drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 6 --
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 6 --
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 6 --
drivers/gpu/drm/sti/sti_drm_crtc.c | 6 --
drivers/gpu/drm/tegra/dc.c | 6 --
include/drm/drm_crtc_helper.h | 6 --
10 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index f69b92535505..8b8fe3762ca9 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -239,7 +239,8 @@ static int atmel_hlcdc_crtc_atomic_check(struct drm_crtc *c,
return atmel_hlcdc_plane_prepare_disc_area(s);
}
-static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c)
+static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc *c,
+ struct drm_crtc_state *old_s)
{
struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c);
@@ -253,7 +254,8 @@ static void atmel_hlcdc_crtc_atomic_begin(struct drm_crtc
*c)
}
}
-static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc)
+static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_s)
{
/* TODO: write common plane control register if available */
}
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 536ae4da4665..50aef1d937e5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1144,7 +1144,7 @@ void drm_atomic_helper_commit_planes(struct drm_device
*dev,
if (!funcs || !funcs->atomic_begin)
continue;
- funcs->atomic_begin(crtc);
+ funcs->atomic_begin(crtc, old_crtc_state);
}
for_each_plane_in_state(old_state, plane, old_plane_state, i) {
@@ -1174,7 +1174,7 @@ void drm_atomic_helper_commit_planes(struct drm_device
*dev,
if (!funcs || !funcs->atomic_flush)
continue;
- funcs->atomic_flush(crtc);
+ funcs->atomic_flush(crtc, old_crtc_state);
}
}
EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
@@ -1210,7 +1210,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct
drm_crtc_state *old_crtc_state)
crtc_funcs = crtc->helper_private;
if (crtc_funcs && crtc_funcs->atomic_begin)
- crtc_funcs->atomic_begin(crtc);
+ crtc_funcs->atomic_begin(crtc, old_crtc_state);
drm_for_each_plane_mask(plane, crtc->dev, plane_mask) {
struct drm_plane_state *old_plane_state =
@@ -1233,7 +1233,7 @@ drm_atomic_helper_commit_planes_on_crtc(struct
drm_crtc_state *old_crtc_state)
}
if (crtc_funcs && crtc_funcs->atomic_flush)
- crtc_funcs->atomic_flush(crtc);
+ crtc_funcs->atomic_flush(crtc, old_crtc_state);
}
EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
diff --git a/drivers/gpu/drm/drm_plane_helper.c
b/drivers/gpu/drm/drm_plane_helper.c
index 2f0ed11024eb..da27fc6f8e4c 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -436,7 +436,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
for (i = 0; i < 2; i++) {
if (crtc_funcs[i] && crtc_funcs[i]->atomic_begin)
- crtc_funcs[i]->atomic_begin(crtc[i]);
+ crtc_funcs[i]->atomic_begin(crtc[i], crtc[i]->state);
}
/*
@@ -451,7 +451,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
for (i = 0; i < 2; i++) {
if (crtc_funcs[i] && crtc_funcs[i]->atomic_flush)
- crtc_funcs[i]->atomic_flush(crtc[i]);
+ crtc_funcs[i]->atomic_flush(crtc[i], crtc[i]->state);
}
/*
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index afe91a8f7e36..599c3d078faa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -103,8 +103,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_state *pipe_config);
static void chv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_state *pipe_config);
-static void intel_beg
[Intel-gfx] [PATCH 1/4] drm/i915: Do not use atomic modesets in hw readout.
This should fix fallout caused by making intel_crtc_control
and update_dpms atomic, which became a problem after reverting the
atomic hw readout patch.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929
Reported-by: Ville Syrjälä
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 75 +++-
1 file changed, 32 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5cc2263db199..7abaffeda7ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6187,31 +6187,35 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
mutex_unlock(&dev->struct_mutex);
}
+static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->dev);
+ enum intel_display_power_domain domain;
+ unsigned long domains;
+
+ if (!intel_crtc->active)
+ return;
+
+ intel_crtc_disable_planes(crtc);
+ dev_priv->display.crtc_disable(crtc);
+
+ domains = intel_crtc->enabled_power_domains;
+ for_each_power_domain(domain, domains)
+ intel_display_power_put(dev_priv, domain);
+ intel_crtc->enabled_power_domains = 0;
+}
+
/*
* turn all crtc's off, but do not adjust state
* This has to be paired with a call to intel_modeset_setup_hw_state.
*/
void intel_display_suspend(struct drm_device *dev)
{
- struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_crtc *crtc;
- for_each_crtc(dev, crtc) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- enum intel_display_power_domain domain;
- unsigned long domains;
-
- if (!intel_crtc->active)
- continue;
-
- intel_crtc_disable_planes(crtc);
- dev_priv->display.crtc_disable(crtc);
-
- domains = intel_crtc->enabled_power_domains;
- for_each_power_domain(domain, domains)
- intel_display_power_put(dev_priv, domain);
- intel_crtc->enabled_power_domains = 0;
- }
+ for_each_crtc(dev, crtc)
+ intel_crtc_disable_noatomic(crtc);
}
/* Master function to enable/disable CRTC and corresponding power wells */
@@ -15120,7 +15124,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_encoder *encoder;
u32 reg;
+ bool enable;
/* Clear any frame start delays used for debugging left by the BIOS */
reg = PIPECONF(crtc->config->cpu_transcoder);
@@ -15137,7 +15143,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
* disable the crtc (and hence change the state) if it is wrong. Note
* that gen4+ has a fixed plane -> pipe mapping. */
if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) {
- struct intel_connector *connector;
bool plane;
DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n",
@@ -15149,29 +15154,8 @@ static void intel_sanitize_crtc(struct intel_crtc
*crtc)
plane = crtc->plane;
to_intel_plane_state(crtc->base.primary->state)->visible = true;
crtc->plane = !plane;
- intel_crtc_control(&crtc->base, false);
+ intel_crtc_disable_noatomic(&crtc->base);
crtc->plane = plane;
-
- /* ... and break all links. */
- for_each_intel_connector(dev, connector) {
- if (connector->encoder->base.crtc != &crtc->base)
- continue;
-
- connector->base.dpms = DRM_MODE_DPMS_OFF;
- connector->base.encoder = NULL;
- }
- /* multiple connectors may have the same encoder:
-* handle them and break crtc link separately */
- for_each_intel_connector(dev, connector)
- if (connector->encoder->base.crtc == &crtc->base) {
- connector->encoder->base.crtc = NULL;
- connector->encoder->connectors_active = false;
- }
-
- WARN_ON(crtc->active);
- crtc->base.state->enable = false;
- crtc->base.state->active = false;
- crtc->base.enabled = false;
}
if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
@@ -15185,13 +15169,18 @@ static void intel_sanitize_crtc(struct intel_crtc
*crtc)
/* Adjust the state of the output pipe according to whether we
* have active connectors/encoders. */
- intel_crtc_update_dpms(&c
[Intel-gfx] [PATCH 4/4] drm/i915: Only enable cursor if it can be enabled.
The cursor should only be enabled if it's visible. This fixes igt/kms_cursor_crc, which may otherwise produce the following warning: [ cut here ] WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() Missing switch case (0) in i9xx_update_cursor Modules linked in: i915 CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: GW 4.1.0-rc7-patser+ #4079 Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 c01aad10 8800b083faa8 817f7827 8001 8800b083faf8 8800b083fae8 81084955 8800b083fad8 8800c4931148 0120 8800c48b Call Trace: [] dump_stack+0x4f/0x7b [] warn_slowpath_common+0x85/0xc0 [] warn_slowpath_fmt+0x41/0x50 [] intel_crtc_update_cursor+0x14c/0x4d0 [i915] [] __intel_set_mode+0x6c4/0x750 [i915] [] intel_crtc_set_config+0x473/0x5c0 [i915] [] drm_mode_set_config_internal+0x69/0x120 [] drm_mode_setcrtc+0x189/0x540 [] drm_ioctl+0x1a0/0x6a0 [] ? get_parent_ip+0x11/0x50 [] do_vfs_ioctl+0x2f8/0x530 [] ? trace_hardirqs_on+0xd/0x10 [] ? selinux_file_ioctl+0x56/0x100 [] SyS_ioctl+0x81/0xa0 [] system_call_fastpath+0x12/0x6f ---[ end trace abf0f71163290a96 ]--- Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 14ccf49b9067..afe91a8f7e36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) intel_enable_primary_hw_plane(crtc->primary, crtc); intel_enable_sprite_planes(crtc); - intel_crtc_update_cursor(crtc, true); + if (to_intel_plane_state(crtc->cursor->state)->visible) + intel_crtc_update_cursor(crtc, true); intel_post_enable_primary(crtc); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: Set hwmode during readout.
This was introduced after converting hw readout to atomic, so it should have been part of the revert too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929 Reported-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cdf6549c8e74..14ccf49b9067 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15357,6 +15357,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) crtc->base.state->enable = crtc->active; crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.hwmode = crtc->config->base.adjusted_mode; plane_state = to_intel_plane_state(primary->state); plane_state->visible = primary_get_hw_state(crtc); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] Fix more fallout from reverting atomic hw readout.
Commit f662af8c5c1619 reverts "drm/i915: Read hw state into an atomic state struct, v2." but it doesn't take into account other changes that were done in that time. Before I continue on the atomic fixes I want to fix the fallout first, and some of the reasons I identified that could cause a failure for atomic modeset. When that's fixed I'll look at committing the atomic hw readout patch again, since it will be needed for converting to full atomic. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929 Maarten Lankhorst (4): drm/i915: Do not use atomic modesets in hw readout. drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip drm/i915: Set hwmode during readout. drm/i915: Only enable cursor if it can be enabled. drivers/gpu/drm/i915/intel_display.c | 108 --- 1 file changed, 63 insertions(+), 45 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip
Use a full atomic call instead. intel_crtc_page_flip will still
have to live until async updates are allowed.
This doesn't seem to be a regression from the convert to atomic,
part 3 patch. During GPU reset it fixes the following warning:
[ cut here ]
WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337
drm_mode_page_flip_ioctl+0x27b/0x360()
Modules linked in: i915
CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090
Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
81c90866 8800d87c3ca8 817f7d87 8001
8800d87c3ce8 81084955 8800
8800d87c3dc0 8800d93d1208 8800b7d1f3e0
Call Trace:
[] dump_stack+0x4f/0x7b
[] warn_slowpath_common+0x85/0xc0
[] warn_slowpath_null+0x15/0x20
[] drm_mode_page_flip_ioctl+0x27b/0x360
[] drm_ioctl+0x1a0/0x6a0
[] ? get_parent_ip+0x11/0x50
[] ? avc_has_perm+0x20/0x280
[] ? get_parent_ip+0x11/0x50
[] do_vfs_ioctl+0x2f8/0x530
[] ? expand_files+0x261/0x270
[] ? selinux_file_ioctl+0x56/0x100
[] SyS_ioctl+0x81/0xa0
[] system_call_fastpath+0x12/0x6f
---[ end trace 9ce834560085bd64 ]---
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/i915/intel_display.c | 29 -
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7abaffeda7ce..cdf6549c8e74 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11618,8 +11618,35 @@ free_work:
kfree(work);
if (ret == -EIO) {
+ struct drm_atomic_state *state;
+ struct drm_plane_state *plane_state;
+
out_hang:
- ret = intel_plane_restore(primary);
+ state = drm_atomic_state_alloc(dev);
+ if (!state)
+ return -ENOMEM;
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+
+retry:
+ plane_state = drm_atomic_get_plane_state(state, primary);
+ ret = PTR_ERR_OR_ZERO(plane_state);
+ if (!ret) {
+ drm_atomic_set_fb_for_plane(plane_state, fb);
+
+ ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
+ if (!ret)
+ ret = drm_atomic_commit(state);
+ }
+
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(state->acquire_ctx);
+ drm_atomic_state_clear(state);
+ goto retry;
+ }
+
+ if (ret)
+ drm_atomic_state_free(state);
+
if (ret == 0 && event) {
spin_lock_irq(&dev->event_lock);
drm_send_vblank_event(dev, pipe, event);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bxt: fix DDI PHY vswing scale value setting
On Thu, 11 Jun 2015, Damien Lespiau wrote: > On Wed, Jun 10, 2015 at 09:18:29AM -0700, Matt Roper wrote: >> On Thu, Jun 04, 2015 at 06:01:35PM +0300, Imre Deak wrote: >> > According to bspec the DDI PHY vswing scale value is "don't care" in >> > case the scale enable bit [27] is clear. But this doesn't seem to be >> > correct. The scale value seems to also matter if the scale mode bit >> > [26] is set. So both bit 26 and 27 depend on the value. Setting the >> > scale value to 0 while either bit is set results in a failed modeset on >> > HDMI (sink reports no signal). >> > >> > After reset the scale value is 0x98, but according to the spec we have >> > to program it to 0x9a. So for consistency program it always to 0x9a >> > regardless of the scale enable bit. >> > >> > Signed-off-by: Imre Deak >> >> This patch successfully enables HDMI display for my team. >> >> Tested-by: Matt Roper > > I believe this should be enough of a good reason for merging at this > stage. > > Acked-by: Damien Lespiau Pushed to drm-intel-next-queued, thanks for the patch and testing/ack. BR, Jani. > > -- > Damien > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915/skl: Retrieve the Rpe value from Pcode
On Wed, 10 Jun 2015, "O'Rourke, Tom" wrote: >> > + >> > + dev_priv->rps.efficient_freq *= >> > + (IS_SKYLAKE(dev) ? GEN9_FREQ_SCALER : 1); > > This line seems awkward. I suppose a good compiler could > optimize out the multiply by one. > > I would prefer something like: > > if(IS_SKYLAKE(dev)) > dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER; Agreed, Jani. > > -- Tom O'Rourke > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Don't set enabled value of all CRTCs when restoring the mode
The code in intel_crtc_restore_mode() sets the enabled value of all the
CRTCs when restoring the mode after a suspend/resume cycle. When more
than one CRTC is enabled, that causes drm_atomic_helper_check_modeset()
to fail if there is more than one pipe enabled, since only one CRTC has
valid connector data. Instead, set only the enabled value for the CRTC
passed as an argument.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90468
References: https://bugs.freedesktop.org/show_bug.cgi?id=90396
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 49c6698..736e653 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12683,7 +12683,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
struct drm_atomic_state *state;
- struct intel_crtc *intel_crtc;
struct intel_encoder *encoder;
struct intel_connector *connector;
struct drm_connector_state *connector_state;
@@ -12726,24 +12725,18 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
}
}
- for_each_intel_crtc(dev, intel_crtc) {
- if (intel_crtc->new_enabled == intel_crtc->base.enabled)
- continue;
-
- crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
- if (IS_ERR(crtc_state)) {
- DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
- intel_crtc->base.base.id,
- PTR_ERR(crtc_state));
- continue;
- }
+ crtc_state = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
+ if (IS_ERR(crtc_state)) {
+ DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
+ crtc->base.id, PTR_ERR(crtc_state));
+ /* FIXME: leaking drm atomic state */
+ return;
+ }
- crtc_state->base.active = crtc_state->base.enable =
- intel_crtc->new_enabled;
+ crtc_state->base.active = crtc_state->base.enable =
+ to_intel_crtc(crtc)->new_enabled;
- if (&intel_crtc->base == crtc)
- drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
- }
+ drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
crtc->primary->fb, crtc->x, crtc->y);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Couple of bug fixes for drm-intel-next-fixes
Hi, These three patches solve two issues in drm-intel-next-fixes. The most important one is the fail to restore modes properly in the force_restore path, after a suspend/resume cycle, fixed by the last two patches. The other is a fix a sent before, for the multiple checks during force restore. Thanks, Ander Ander Conselvan de Oliveira (3): drm/i915: Don't check modeset state in the hw state force restore path drm/i915: Don't update staged config in during force restore modesets drm/i915: Don't set enabled value of all CRTCs when restoring the mode drivers/gpu/drm/i915/intel_display.c | 54 1 file changed, 24 insertions(+), 30 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Don't update staged config during force restore modesets
The force restore path relies on the staged config to preserve the
configuration used before a suspend/resume cycle. The update done to it
in intel_modeset_fixup_state() would cause that information to be lost
after the first modeset, making it impossible to restore the modes for
pipes B and C.
References: https://bugs.freedesktop.org/show_bug.cgi?id=90468
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6ef57e6..49c6698 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11386,10 +11386,6 @@ static void intel_modeset_fixup_state(struct
drm_atomic_state *state)
crtc->base.enabled = crtc->base.state->enable;
crtc->config = to_intel_crtc_state(crtc->base.state);
}
-
- /* Copy the new configuration to the staged state, to keep the few
-* pieces of code that haven't been converted yet happy */
- intel_modeset_update_staged_output_state(state->dev);
}
static void
@@ -12654,8 +12650,10 @@ static int intel_set_mode_with_config(struct drm_crtc
*crtc,
ret = __intel_set_mode(crtc, pipe_config);
- if (ret == 0 && check)
+ if (ret == 0 && check) {
+ intel_modeset_update_staged_output_state(crtc->dev);
intel_modeset_check_state(crtc->dev);
+ }
return ret;
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Don't check modeset state in the hw state force restore path
Since the force restore logic will restore the CRTCs state one at a
time, it is possible that the state will be inconsistent until the whole
operation finishes. A call to intel_modeset_check_state() is done once
it's over, so don't check the state multiple times in between. This
regression was introduced in:
commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
Author: Jesse Barnes
Date: Wed Nov 5 14:26:06 2014 -0800
drm/i915: factor out compute_config from __intel_set_mode v3
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
Cc: Jesse Barnes
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4e3f302..6ef57e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -87,7 +87,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_atomic_state *state);
+ struct drm_atomic_state *state,
+ bool check);
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -10096,7 +10097,7 @@ retry:
drm_mode_copy(&crtc_state->base.mode, mode);
- if (intel_set_mode(crtc, state)) {
+ if (intel_set_mode(crtc, state, true)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
@@ -10170,7 +10171,7 @@ void intel_release_load_detect_pipe(struct
drm_connector *connector,
if (ret)
goto fail;
- ret = intel_set_mode(crtc, state);
+ ret = intel_set_mode(crtc, state, true);
if (ret)
goto fail;
@@ -12646,20 +12647,22 @@ static int __intel_set_mode(struct drm_crtc
*modeset_crtc,
}
static int intel_set_mode_with_config(struct drm_crtc *crtc,
- struct intel_crtc_state *pipe_config)
+ struct intel_crtc_state *pipe_config,
+ bool check)
{
int ret;
ret = __intel_set_mode(crtc, pipe_config);
- if (ret == 0)
+ if (ret == 0 && check)
intel_modeset_check_state(crtc->dev);
return ret;
}
static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_atomic_state *state)
+ struct drm_atomic_state *state,
+ bool check)
{
struct intel_crtc_state *pipe_config;
int ret = 0;
@@ -12670,7 +12673,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
goto out;
}
- ret = intel_set_mode_with_config(crtc, pipe_config);
+ ret = intel_set_mode_with_config(crtc, pipe_config, check);
if (ret)
goto out;
@@ -12747,7 +12750,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
crtc->primary->fb, crtc->x, crtc->y);
- ret = intel_set_mode(crtc, state);
+ ret = intel_set_mode(crtc, state, false);
if (ret)
drm_atomic_state_free(state);
}
@@ -12947,7 +12950,7 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
primary_plane_was_visible = primary_plane_visible(set->crtc);
- ret = intel_set_mode_with_config(set->crtc, pipe_config);
+ ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
if (ret == 0 &&
pipe_config->base.enable &&
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] Limit CHV max cdclk
For Cherryview the CD clock is limited up to 320MHz. Based on the received comments, I cleaned up the if-else tree. Mika Kahola (1): drm/i915: Limit CHV max cdclk drivers/gpu/drm/i915/intel_display.c | 2 ++ 1 file changed, 2 insertions(+) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] drm/i915: Limit CHV max cdclk
Limit CHV maximum cdclk to 320MHz.
v2: Rebase to the latest
v3: Clean up of if-else tree
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5cc2263..c027012 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5256,6 +5256,8 @@ static void intel_update_max_cdclk(struct drm_device *dev)
dev_priv->max_cdclk_freq = 54;
else
dev_priv->max_cdclk_freq = 675000;
+ } else if (IS_CHERRYVIEW(dev)) {
+ dev_priv->max_cdclk_freq = 32;
} else if (IS_VALLEYVIEW(dev)) {
dev_priv->max_cdclk_freq = 40;
} else {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
