Re: [Intel-gfx] [PATCH 1/2] drm/i915: Allow parsing of variable size child device entries from VBT
On Mon, Jul 13, 2015 at 11:24:46AM +0200, Daniel Vetter wrote: > On Fri, Jul 10, 2015 at 02:10:54PM +0300, Antti Koskipaa wrote: > > VBT version 196 increased the size of common_child_dev_config. The parser > > code assumed that the size of this structure would not change. > > > > So now, instead of checking for smaller size, check that the VBT entry is > > not too large and memcpy only child_dev_size amount of data, leaving any > > trailing entries as zero. If this is not good enough for the future, > > we can always sprinkle extra version checks in there. > > > > Signed-off-by: Antti Koskipaa > > As I mentioned in the other threads I think with vbt it's not too paranoid > to double-check our assumptions. So for each vbt version range I'd like us > to check what size we exactly expect. Being super paranoid with vbt is imo > good practice since otherwise the hw teams will sneak in another update > without us realizing it. Antti's on vacation now for the next few weeks. Do you need these modifications as a pre-requisite for merging his patch, or can further improvements be submitted separately? Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: manual merge of the drm-intel tree with the drm-intel-fixes tree
On Tue, Jul 14, 2015 at 12:11:12PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the drm-intel tree got a conflict in:
>
> drivers/gpu/drm/i915/intel_display.c
>
> between commit:
>
> 8aa3053bf731 ("drm/i915: fix oops in primary_check_plane")
>
> from the drm-intel-fixes tree and commit:
>
> da20eabd2c69 ("drm/i915: Split plane updates of crtc->atomic into a helper,
> v2.")
>
> from the drm-intel tree.
>
> I fixed it up (but it probably needs more - see below) and can carry
> the fix as necessary.
>
> Daniel, can you please merge your fixes branch into your main branch
> (maybe after Linus has merged it) and fix these conflicts correctly as
> these conflicts tend to go on and on as the files get changed.
Well the problem is that they indeed go on and on and we still change them
so they resurface all the time. I've done a few backmerges in my next
branch already, I plan to do the next backmerge somewhen this week.
Sorry about all the pain this is causing.
-Daniel
>
> --
> Cheers,
> Stephen Rothwells...@canb.auug.org.au
>
> diff --cc drivers/gpu/drm/i915/intel_display.c
> index 85ac6d85dc39,00c60c1c5162..
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@@ -4851,25 -4802,13 +4802,16 @@@ static void intel_crtc_disable_planes(s
> {
> struct drm_device *dev = crtc->dev;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_plane *intel_plane;
> + struct drm_plane *p;
> int pipe = intel_crtc->pipe;
>
> +if (!intel_crtc->active)
> +return;
> +
> - intel_crtc_wait_for_pending_flips(crtc);
> -
> - intel_pre_disable_primary(crtc);
> -
> intel_crtc_dpms_overlay_disable(intel_crtc);
> - for_each_intel_plane(dev, intel_plane) {
> - if (intel_plane->pipe == pipe) {
> - struct drm_crtc *from = intel_plane->base.crtc;
>
> - intel_plane->disable_plane(&intel_plane->base,
> -from ?: crtc, true);
> - }
> - }
> + drm_for_each_plane_mask(p, dev, plane_mask)
> + to_intel_plane(p)->disable_plane(p, crtc);
>
> /*
>* FIXME: Once we grow proper nuclear flip support out of this we need
> @@@ -13382,47 -13751,11 +13757,14 @@@ static void intel_begin_crtc_commit(str
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_plane *intel_plane;
> - struct drm_plane *p;
> - unsigned fb_bits = 0;
> -
> - /* Track fb's for any planes being disabled */
> - list_for_each_entry(p, &dev->mode_config.plane_list, head) {
> - intel_plane = to_intel_plane(p);
> -
> - if (intel_crtc->atomic.disabled_planes &
> - (1 << drm_plane_index(p))) {
> - switch (p->type) {
> - case DRM_PLANE_TYPE_PRIMARY:
> - fb_bits =
> INTEL_FRONTBUFFER_PRIMARY(intel_plane->pipe);
> - break;
> - case DRM_PLANE_TYPE_CURSOR:
> - fb_bits =
> INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe);
> - break;
> - case DRM_PLANE_TYPE_OVERLAY:
> - fb_bits =
> INTEL_FRONTBUFFER_SPRITE(intel_plane->pipe);
> - break;
> - }
>
> - mutex_lock(&dev->struct_mutex);
> - i915_gem_track_fb(intel_fb_obj(p->fb), NULL, fb_bits);
> - mutex_unlock(&dev->struct_mutex);
> - }
> - }
> -
> - if (intel_crtc->atomic.wait_for_flips)
> - intel_crtc_wait_for_pending_flips(crtc);
> -
> - if (intel_crtc->atomic.disable_fbc)
> - intel_fbc_disable(dev);
> + if (!needs_modeset(crtc->state))
> + intel_pre_plane_update(intel_crtc);
>
> +if (intel_crtc->atomic.disable_ips)
> +hsw_disable_ips(intel_crtc);
> +
> - if (intel_crtc->atomic.pre_disable_primary)
> - intel_pre_disable_primary(crtc);
> -
> - if (intel_crtc->atomic.update_wm)
> + if (intel_crtc->atomic.update_wm_pre)
> intel_update_watermarks(crtc);
>
> intel_runtime_pm_get(dev_priv);
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: build failure after merge of the drm-misc tree
On Tue, Jul 14, 2015 at 10:30:56AM +0530, Archit Taneja wrote:
>
> Hi,
>
> On 07/14/2015 08:22 AM, Stephen Rothwell wrote:
> >Hi all,
> >
> >After merging the drm-misc tree, today's linux-next build (x86_64
> >allmodconfig) failed like this:
> >
> >drivers/gpu/drm/virtio/virtgpu_drm_bus.c: In function
> >'virtio_pci_kick_out_firmware_fb':
> >drivers/gpu/drm/virtio/virtgpu_drm_bus.c:55:2: error: implicit declaration
> >of function 'drm_fb_helper_remove_conflicting_framebuffers'
> >[-Werror=implicit-function-declaration]
> > drm_fb_helper_remove_conflicting_framebuffers(ap, "virtiodrmfb",
> > ^
> >
> >Caused by commit
> >
> > 7bd870e7b1c8 ("drm/virtio: Use new drm_fb_helper functions")
> >
> >I have used the drm-misc tree from next-20150713 for today.
> >
> >(That commit said "COMPILE TESTED ONLY" :-()
>
> My bad. The commit messages were for a slightly older version.
> I'll fix this, and the warnings in the other mail.
Yeah it's a big patch series with a testing chicken-egg problem, so I
figured I'll give it a spin in -next. Dropped again until it works better.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] 4.2-rc2 regression: drm/i915
Can you please attach your Xorg.log? Thanks, Daniel On Tue, Jul 14, 2015 at 6:51 AM, Harald Arnesen wrote: > 4.2-rc2 gives me an unusable X11 screen (se attached picture). > 4.2-rc1 is OK. > > Bisected to the following: > > > 19ee835cdb0b5a8eb11a68f25a51b8039d564488 is the first bad commit > commit 19ee835cdb0b5a8eb11a68f25a51b8039d564488 > Author: Chris Wilson > Date: Mon Jun 29 14:01:19 2015 +0100 > > drm/i915: Declare the swizzling unknown for L-shaped configurations > > The old style of memory interleaving swizzled upto the end of the > first even bank of memory, and then used the remainder as unswizzled on > the unpaired bank - i.e. swizzling is not constant for all memory. This > causes problems when we try to migrate memory and so the kernel prevents > migration at all when we detect L-shaped inconsistent swizzling. > However, this issue also extends to userspace who try to manually detile > into memory as the swizzling for an individual page is unknown (it > depends on its physical address only known to the kernel), userspace > cannot correctly swizzle objects. > > v2: Mark the global swizzling as unknown rather than adjust the value > reported to userspace. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91105 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter > > :04 04 a34dedabcb878bfc55ce76c231d60fc801f79319 > 3e45d34152a15df5cf07fa50e6720c9948ed8a6a M drivers > > > > And indeed, if I revert this commit, 4.2-rc2 is OK as well. > Kernel config and dmesg from rc1 and rc2 is attached. > -- > Hilsen Harald -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Adjust BXT HDMI port clock limits
On Mon, Jul 13, 2015 at 05:32:14PM +, Bish, Jim wrote: > On Mon, 2015-07-13 at 11:05 +0200, Daniel Vetter wrote: > > On Fri, Jul 10, 2015 at 02:27:48PM +0100, Damien Lespiau wrote: > > > On Fri, Jul 10, 2015 at 04:21:27PM +0300, Ville Syrjälä wrote: > > > > On Fri, Jul 10, 2015 at 02:18:57PM +0100, Damien Lespiau wrote: > > > > > On Fri, Jul 10, 2015 at 04:09:42PM +0300, Imre Deak wrote: > > > > > > On ma, 2015-07-06 at 14:44 +0300, ville.syrj...@linux.intel.com > > > > > > wrote: > > > > > > > From: Ville Syrjälä > > > > > > > > > > > > > > Since > > > > > > > commit e62925567c7926e78bc8ca976cde5c28ea265a49 > > > > > > > Author: Vandana Kannan > > > > > > > Date: Wed Jul 1 17:02:57 2015 +0530 > > > > > > > > > > > > > > drm/i915/bxt: BUNs related to port PLL > > > > > > > > > > > > > > BXT DPLL can now generate frequencies in the 216-223 MHz range. > > > > > > > Adjust the HDMI port clock checks to account for the reduced range > > > > > > > of invalid frequencies. > > > > > > > > > > > > > > Cc: Vandana Kannan > > > > > > > Cc: Imre Deak > > > > > > > Signed-off-by: Ville Syrjälä > > > > > > > > > > > > Ville wrote a tool for CHV that calculates the valid frequencies > > > > > > based > > > > > > on the algorithm in the kernel. With the help of that I verified > > > > > > that > > > > > > this matches the list of target frequencies bxt_find_best_dpll() > > > > > > will > > > > > > accept, so: > > > > > > > > > > Could we have that tool in i-g-t? > > > > > > > > We could lift all the .find_dpll routines from the kernel into i-g-t. > > > > The only real concern is that we'll forget to update the i-g-t copies > > > > when changing the kernel. But I guess it would still be easier to just > > > > update them slightly when noticing that rather than having to lift them > > > > from the kernel all over again. > > > > > > Right, while not ideal, I think having something in i-g-t, even if it > > > diverges slightly (but then we can remind the patch author to update the > > > i-g-t tool during review) is still better than not having that code > > > around at all. > > > > Another way to check this would be to inject EDIDs with hand-rolled > > timings that increment in small steps. Then we can try modesets on all the > > unfiltered ones vs. just manually adding it with addmode. If any mode gets > > filtered inconsistently in the mode list parsing compared to modeset code > > that would be a bug. > > > > Unfortunately the hdmi injection stuff isn't ready yet. I'll create a jira > > for this idea. > > -Daniel > > if you have dr. HDMI device you can set custom EDID in slots 6 and 7 - > works great for this type of exercise with no additional necessary to > try out. The idea is to be able to run the testsuite with bare-bones machines and no need to send each developer a special piece of hw for each type of output. I know that there are tons of hw solutions for these testing problems, they don't seem to scale well enough. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check live status before reading edid
On Tue, Jul 14, 2015 at 10:16:17AM +0530, Jindal, Sonika wrote:
>
>
> On 7/13/2015 8:25 PM, Daniel Vetter wrote:
> >On Mon, Jul 13, 2015 at 04:35:00PM +0530, Sonika Jindal wrote:
> >>Adding this for SKL onwards.
> >>
> >>v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> >>to check digital port status. Adding a separate function to get bxt live
> >>status (Daniel)
> >>
> >>Signed-off-by: Sonika Jindal
> >>---
> >> drivers/gpu/drm/i915/intel_dp.c |4 ++--
> >> drivers/gpu/drm/i915/intel_drv.h |2 ++
> >> drivers/gpu/drm/i915/intel_hdmi.c | 42
> >> +
> >> 3 files changed, 42 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >>b/drivers/gpu/drm/i915/intel_dp.c
> >>index 4ebfc3a..7b54b9d 100644
> >>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>@@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> >>return intel_dp_detect_dpcd(intel_dp);
> >> }
> >>
> >>-static int g4x_digital_port_connected(struct drm_device *dev,
> >>- struct intel_digital_port
> >>*intel_dig_port)
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+ struct intel_digital_port *intel_dig_port)
> >> {
> >>struct drm_i915_private *dev_priv = dev->dev_private;
> >>uint32_t bit;
> >>diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >>b/drivers/gpu/drm/i915/intel_drv.h
> >>index a47fbc3..30c8dd6 100644
> >>--- a/drivers/gpu/drm/i915/intel_drv.h
> >>+++ b/drivers/gpu/drm/i915/intel_drv.h
> >>@@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp
> >>*intel_dp);
> >> void intel_edp_drrs_invalidate(struct drm_device *dev,
> >>unsigned frontbuffer_bits);
> >> void intel_edp_drrs_flush(struct drm_device *dev, unsigned
> >> frontbuffer_bits);
> >>+int g4x_digital_port_connected(struct drm_device *dev,
> >>+ struct intel_digital_port *intel_dig_port);
> >>
> >> /* intel_dp_mst.c */
> >> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port,
> >> int conn_id);
> >>diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> >>b/drivers/gpu/drm/i915/intel_hdmi.c
> >>index 1fb6919..7eb0d0e 100644
> >>--- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>+++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>@@ -1300,6 +1300,39 @@ intel_hdmi_unset_edid(struct drm_connector
> >>*connector)
> >>to_intel_connector(connector)->detect_edid = NULL;
> >> }
> >>
> >>+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> >>+ struct intel_digital_port *port)
> >>+{
> >>+ u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> >>+
> >>+ switch (port->port) {
> >>+ case PORT_B:
> >>+ return temp & BXT_DE_PORT_HP_DDIB;
> >>+
> >>+ case PORT_C:
> >>+ return temp & BXT_DE_PORT_HP_DDIC;
> >>+
> >>+ default:
> >>+ return false;
> >>+
> >>+ }
> >>+}
> >>+
> >>+static bool intel_hdmi_live_status(struct intel_digital_port
> >>*intel_dig_port)
> >>+{
> >>+ struct drm_device *dev = intel_dig_port->base.base.dev;
> >>+ struct drm_i915_private *dev_priv = to_i915(dev);
> >>+
> >>+ if (IS_VALLEYVIEW(dev))
> >>+ return g4x_digital_port_connected(dev, intel_dig_port);
> >>+ else if (IS_SKYLAKE(dev))
> >>+ return ibx_digital_port_connected(dev_priv, intel_dig_port);
> >>+ else if (IS_BROXTON(dev))
> >>+ return bxt_port_connected(dev_priv, intel_dig_port);
> >
> >Really I want this to be rolled out for all platforms, together with the
> >fix for handling hpd interrupts. Together with a reference to the old
> >commits we had to revert because it didn't work.
> >
> >I want to test this on my ivb (since that's the machine where I can
> >readily reproduce this bug), and if it still doesn't work there I won't
> >take this.
> >-Daniel
> Is there a formal process to raise a test for hpd on all platforms which
> might be affected by this?
Get it merged and wait for the regression reports to come in or not. The
entire problem I'm trying to explain here is that these hpd problems where
_not_ detected internally here at Intel, but only reported by external
people. Only later on did I come across a machine (by accident) which
seems to exhibit the same problem.
That's also why I want to enable it everywhere, increases the chances
we'll get a negative report if it doesn't work.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect
On Tue, Jul 14, 2015 at 03:48:36AM +, Sharma, Shashank wrote: > Hi Daniel, Chris > > Gen7 and Gen8 platforms have a different live status register (0x61114) and > we need not to rely on ISR on that. > As Sonika mentioned, We have been using this register for our commercial > releases, and found them reliable across a wide range of monitors. > > On the other hand, Bsepc clearly mentions, to check the live status before > even try to read EDID. > The current DRM nightly code doesn't do that, and we have received few errors > from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > So I think this patch and solution is ready, and it should go in. I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? -Daniel > > Regards > Shashank > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Monday, July 13, 2015 8:27 PM > To: Jindal, Sonika > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from > detect > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > >>During init_connector set the edid, then edid will be set/unset only > > >>during hotplug. For the sake of older platforms where HPD is not > > >>stable, let edid read happen from detect as well only if it is forced to > > >>do so. > > >> > > >>v2: Removing the 'force' check, instead let detect call read the > > >>edid for platforms older than gen 7 (Shashank) > > > > > >That's enough worse. We now have a random gen check with no rationale > > >for why you suddenly believe all manufacturers have fixed their wiring. > > >There is no reason to believe that gen7 suddenly works is there? If > > >there is, why don't you mention it? > > >-Chris > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > For CHV/VLV, already the live status check is stable enough to > > determine if we can read edid or not. In VPG production branches we > > have this patch already available and it was tested with variety of > > monitors extensively. So we now read the edid only during init and during > > hotplug. > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > patch makes HPD stable enough. > > So, we can rely on the edid read from init_connector instead of > > reading edid on every detect call. > > Again, not going to take this with random gen checks. I want your fix for > handling hpd on other platforms, then roll this out everywhere. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ 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: Allow parsing of variable size child device entries from VBT
On Tue, Jul 14, 2015 at 10:11:37AM +0300, David Weinehall wrote: > On Mon, Jul 13, 2015 at 11:24:46AM +0200, Daniel Vetter wrote: > > On Fri, Jul 10, 2015 at 02:10:54PM +0300, Antti Koskipaa wrote: > > > VBT version 196 increased the size of common_child_dev_config. The parser > > > code assumed that the size of this structure would not change. > > > > > > So now, instead of checking for smaller size, check that the VBT entry is > > > not too large and memcpy only child_dev_size amount of data, leaving any > > > trailing entries as zero. If this is not good enough for the future, > > > we can always sprinkle extra version checks in there. > > > > > > Signed-off-by: Antti Koskipaa > > > > As I mentioned in the other threads I think with vbt it's not too paranoid > > to double-check our assumptions. So for each vbt version range I'd like us > > to check what size we exactly expect. Being super paranoid with vbt is imo > > good practice since otherwise the hw teams will sneak in another update > > without us realizing it. > > Antti's on vacation now for the next few weeks. Do you need these > modifications as a pre-requisite for merging his patch, or can further > improvements be submitted separately? This patch starts to make our vbt checking lax. I don't want to walk down this road really, so yes I prefer if we just keep on having really strict checks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 19/20] drm/i915: always disable irqs in intel_pipe_update_start
Op 13-07-15 om 19:16 schreef Daniel Stone: > Hi, > > On 13 July 2015 at 15:30, Maarten Lankhorst > wrote: >> @@ -13649,9 +13647,7 @@ static void intel_begin_crtc_commit(struct drm_crtc >> *crtc) >> >> /* Perform vblank evasion around commit operation */ >> if (crtc->state->active) >> - intel_crtc->atomic.evade = >> - intel_pipe_update_start(intel_crtc, >> - >> &intel_crtc->atomic.start_vbl_count); >> + intel_pipe_update_start(intel_crtc, >> &intel_crtc->atomic.start_vbl_count); >> >> if (!needs_modeset(crtc->state) && INTEL_INFO(dev)->gen >= 9) >> skl_detach_scalers(intel_crtc); >> @@ -13663,9 +13659,8 @@ static void intel_finish_crtc_commit(struct drm_crtc >> *crtc) >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> >> - if (intel_crtc->atomic.evade) >> - intel_pipe_update_end(intel_crtc, >> - intel_crtc->atomic.start_vbl_count); > Can we get rid of the 'evade' member in the struct now? > > Cheers, > Daniel Yeah, it's useless now. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to stop_rings mode bits
On Mon, Jul 13, 2015 at 04:07:14PM +, Gore, Tim wrote: > > > Tim Gore > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > > > > -Original Message- > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > > Vetter > > Sent: Monday, July 13, 2015 3:59 PM > > To: Gore, Tim > > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Wood, Thomas > > Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to > > stop_rings > > mode bits > > > > On Mon, Jul 13, 2015 at 09:43:11AM +, Gore, Tim wrote: > > > > > > > > > Tim Gore > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon > > > SN3 1RJ > > > > > > > -Original Message- > > > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > > > > Daniel Vetter > > > > Sent: Monday, July 13, 2015 10:30 AM > > > > To: Gore, Tim > > > > Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas > > > > Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes > > > > to stop_rings mode bits > > > > > > > > On Fri, Jul 10, 2015 at 02:06:06PM +0100, tim.g...@intel.com wrote: > > > > > From: Tim Gore > > > > > > > > > > In function igt_set_stop_rings, the test > > > > > igt_assert_f(flags == 0 || current == 0, .. > > > > > > > > > > will fail if we are trying to force a hang but the > > > > > STOP_RINGS_ALLOW_BAN or STOP_RINGS_ALLOW_ERROR bit is set. > > > > > With the introduction of per ring resets in the driver (in > > > > > android) these bits do not get cleared to zero when an individual > > > > > ring is reset. This causes subsequent attempt to cause a ring hang > > > > > via this function to fail, leading to several igt tests failing > > > > > (ie gem_reset_stats subtest ban-xxx etc). > > > > > > > > Fix tdr to reset these instead? > > > > -Daniel > > > > > > > I could change tdr, but why. When the TDR handles a ring hang and > > > resets the ring, why would it modify the flag that defines if the > > > driver should ban a frequently hanging context? If we get rid of the > > > stop_rings interface, as Chris Wilson suggested, we would still need > > > to keep the STOP_RING_ALLOW_BAN/ALLOW_ERRORS bits in debugfs, but > > you > > > would not expect to have to re-write these bits each time there is a ring > > reset. > > > > The fix current hang recover code to no reset this, add some grace period, > > then push this patch to igt. We don't have full-blown abi guarantees for > > debugfs/igt stuff, but I want at least a few months (really last released > > kernel&igt) of backwards/forward compatibility. And inconsistent behaviour > > isn't great imo. > > -Daniel > > Sorry Daniel, I didn't really follow that. > I didn't want a gpu reset to clear the ALLOW_BAN bit, since this will defeat > the > point of this bit, except perhaps in test situations where you can keep > setting it > each time you deliberately cause a hang. It seems like the ALLOW_BAN bit has > uses in real world situations, although I don't know it anyone uses it. ALLOW_BAN is only for testing. It's meant to re-enable auto-banning because we disable that by default for automated tests. This is all meant to be used together with the stop_rings stuff only. > Would you be more comfortable with > Igt_assert_f ( ( flags ==0 ) || > (( current & STOP_RING_ALL) ==0) && ((current ^ flags) & ~ > STOP_RING_ALL == 0 ) ) > > So the either the new flags must be 0 (currently allowed) or the existing > flags must > indicate that all hangs are cleared (0 except possibly the mode bits) AND > the mode > bits you are writing are the same as the current values. ?? Iirc gem_reset_stats uses stop_rings only to supress gpu reset warnings in dmesg (since they're expected) and not for the actual stop_rings logic. It sets stop_rings after submitting the hanging batch, but before that one is detected as hung. We could just add another bit to stop_rings for that case. What I still don't understand is why tdr can't just keep on properly resetting stop_rings. It'll break tons of existing tests, and I don't understand the upside. stop_rings has become quasi-abi (that's why the ALLOW bits have such funky semantics, it's for backwards compat), if you need to change it you need to extend it, not break it. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ 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/bxt: WA for swapped HPD pins in A stepping
On Tue, Jul 14, 2015 at 11:18:50AM +0530, Sonika Jindal wrote:
> As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
> and interrupts to check the external panel connection and DDIC HPD
> logic for edp panel.
>
> Signed-off-by: Sonika Jindal
Yeah I think this is much clearer. Will pull in as soon as there's an r-b
on these two.
Thanks, Daniel
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 --
> drivers/gpu/drm/i915/intel_hdmi.c |9 -
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7b54b9d..c32f3d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> /* Set up the hotplug pin. */
> switch (port) {
> case PORT_A:
> - intel_encoder->hpd_pin = HPD_PORT_A;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
> + * interrupts to check the eDP panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_B:
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 44e7160..121e626 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2011,7 +2011,14 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> if (IS_BROXTON(dev_priv))
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect
Please apply this patch series along with the Interrupt handling fix patch Sonika shared. With these 4 patches applied, we should not see any problems with HPDs (Until the HW is broken). On a similar note, the corresponding chrome team applied the live status check patch, along with others, and they are not seeing the problems with HPD, hence they are also interested in this patch series. Please let us know if you observe something else, we would love to dig further. But as we previously mentioned, this patch series is available and running across various MCG Gen7 devices, and available with Gen8 PV production branches also, and there the hotplug stability is pretty good. Regards Shashank -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, July 14, 2015 1:29 PM To: Sharma, Shashank Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect On Tue, Jul 14, 2015 at 03:48:36AM +, Sharma, Shashank wrote: > Hi Daniel, Chris > > Gen7 and Gen8 platforms have a different live status register (0x61114) and > we need not to rely on ISR on that. > As Sonika mentioned, We have been using this register for our commercial > releases, and found them reliable across a wide range of monitors. > > On the other hand, Bsepc clearly mentions, to check the live status before > even try to read EDID. > The current DRM nightly code doesn't do that, and we have received few errors > from Gen7 Chromebooks where you can still read valid EDID on HDMI hot-unplug. > > So I think this patch and solution is ready, and it should go in. I have a gen7 machine here which is currently (and it's somewhat random) broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix for the hpd handling - or did I miss it? -Daniel > > Regards > Shashank > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Monday, July 13, 2015 8:27 PM > To: Jindal, Sonika > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > from detect > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > >>During init_connector set the edid, then edid will be set/unset > > >>only during hotplug. For the sake of older platforms where HPD is > > >>not stable, let edid read happen from detect as well only if it is forced > > >>to do so. > > >> > > >>v2: Removing the 'force' check, instead let detect call read the > > >>edid for platforms older than gen 7 (Shashank) > > > > > >That's enough worse. We now have a random gen check with no > > >rationale for why you suddenly believe all manufacturers have fixed their > > >wiring. > > >There is no reason to believe that gen7 suddenly works is there? If > > >there is, why don't you mention it? > > >-Chris > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > For CHV/VLV, already the live status check is stable enough to > > determine if we can read edid or not. In VPG production branches we > > have this patch already available and it was tested with variety of > > monitors extensively. So we now read the edid only during init and during > > hotplug. > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > patch makes HPD stable enough. > > So, we can rely on the edid read from init_connector instead of > > reading edid on every detect call. > > Again, not going to take this with random gen checks. I want your fix for > handling hpd on other platforms, then roll this out everywhere. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Provide compat ioctl for addfb2.1
On Mon, Jul 13, 2015 at 04:56:53PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Frame buffer modifiers extensions provided in;
>
> commit e3eb3250d84ef97b766312345774367b6a310db8
> Author: Rob Clark
> Date: Thu Feb 5 14:41:52 2015 +
>
> drm: add support for tiled/compressed/etc modifier in addfb2
>
> Missed the structure packing/alignment problem where 64-bit
> members were added after the odd number of 32-bit ones. This
> makes the compiler produce structures of different sizes under
> 32- and 64-bit x86 targets and makes the ioctl need explicit
> compat handling.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: dri-de...@lists.freedesktop.org
> Cc: Rob Clark
> Cc: Daniel Stone
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/drm_ioc32.c | 60
> +
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index aa8bbb460c57..2bea11bfb3ae 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -70,6 +70,8 @@
>
> #define DRM_IOCTL_WAIT_VBLANK32 DRM_IOWR(0x3a,
> drm_wait_vblank32_t)
>
> +#define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8,
> drm_mode_fb_cmd232_t)
> +
> typedef struct drm_version_32 {
> int version_major;/**< Major version */
> int version_minor;/**< Minor version */
> @@ -1016,6 +1018,63 @@ static int compat_drm_wait_vblank(struct file *file,
> unsigned int cmd,
> return 0;
> }
>
> +typedef struct drm_mode_fb_cmd232 {
Please no typedefs for structs in new code. drm_ioc32.c has lots of them
simply because it's old. Also the patch needs cc: stable. With that
Reviewed-by: Daniel Vetter
> + u32 fb_id;
> + u32 width;
> + u32 height;
> + u32 pixel_format;
> + u32 flags;
> + u32 handles[4];
> + u32 pitches[4];
> + u32 offsets[4];
> + u64 modifier[4];
> +} __attribute__((packed)) drm_mode_fb_cmd232_t;
> +
> +static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + drm_mode_fb_cmd232_t __user *argp = (void __user *)arg;
> + struct drm_mode_fb_cmd232 req32;
> + struct drm_mode_fb_cmd2 __user *req64;
> + int i;
> + int err;
> +
> + if (copy_from_user(&req32, argp, sizeof(req32)))
> + return -EFAULT;
> +
> + req64 = compat_alloc_user_space(sizeof(*req64));
> +
> + if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
> + || __put_user(req32.width, &req64->width)
> + || __put_user(req32.height, &req64->height)
> + || __put_user(req32.pixel_format, &req64->pixel_format)
> + || __put_user(req32.flags, &req64->flags))
> + return -EFAULT;
> +
> + for (i = 0; i < 4; i++) {
> + if (__put_user(req32.handles[i], &req64->handles[i]))
> + return -EFAULT;
> + if (__put_user(req32.pitches[i], &req64->pitches[i]))
> + return -EFAULT;
> + if (__put_user(req32.offsets[i], &req64->offsets[i]))
> + return -EFAULT;
> + if (__put_user(req32.modifier[i], &req64->modifier[i]))
> + return -EFAULT;
> + }
> +
> + err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
> + if (err)
> + return err;
> +
> + if (__get_user(req32.fb_id, &req64->fb_id))
> + return -EFAULT;
> +
> + if (copy_to_user(argp, &req32, sizeof(req32)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static drm_ioctl_compat_t *drm_compat_ioctls[] = {
> [DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
> [DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
> @@ -1048,6 +1107,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
> [DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
> #endif
> [DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
> + [DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
> };
>
> /**
> --
> 2.4.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/radeon: Switch to drm_vblank_on/off
On 28.05.2015 18:03, Michel Dänzer wrote: > On 28.05.2015 17:38, Daniel Vetter wrote: >> On Thu, May 28, 2015 at 04:11:53PM +0900, Michel Dänzer wrote: >>> On 27.05.2015 18:41, Daniel Vetter wrote: On Wed, May 27, 2015 at 06:21:24PM +0900, Michel Dänzer wrote: > On 27.05.2015 18:04, Daniel Vetter wrote: >> These should be functionally equivalent to the older per/post modeset >> functions, except that they block out drm_vblank_get right away. >> There's only the clock adjusting code (outside of pageflips) in >> readone which uses drm_vblank_get. But that code doesn't synchronize >> against concurrent modesets and instead handles any such races by >> waiting for the right vblank to arrive with a short timetout. >> >> The longer-term plan here is to switch all kms drivers to >> drm_vblank_on/off so that common code like pending event cleanup can >> be done there, while drm_vblank_pre/post_modeset will be purely >> drm internal for the old UMS ioctl. >> >> Note that the kerneldoc for pre/post_modeset is wrong since as Michel >> Dänzer correctly pointed out it works if only using pre/post_modeset. >> The trouble that lead to this comment is the very old version of >> drm_vblank_off to clear out pending events when disabling a pipe, >> which did seem to wreak havoc with the trick used by pre/post_modeset. >> Michel also expressed dissatisfaction with intel folks pushing new >> interfaces with bogus justifications. I still maintain that having a >> consistent set of vblank behaviour across kms drivers, separate from >> any old UMS functions is a useful goal. >> >> Cc: Michel Dänzer >> Signed-off-by: Daniel Vetter > > Can you describe at least one tangible benefit this change provides for > the radeon driver? > > Because I'm afraid that this might cause subtle breakage, and since we > don't have any rigorous tests for this like in intel-gpu-tools (yet?), > it might be painful to track it down. > > So, I'd like to have a good reason for taking the risk. right now at most a bit of code to clean out pending events on modeset disable, for somewhat consistent behaviour with other drivers. But in general it's fairly ill-defined what happens with vblank events. >>> >>> Yeah, while that's nice to have, I don't think it makes too much >>> difference in practice. >>> >>> Anyway, I'm giving this patch a spin, and it does indeed cause userspace >>> fallout, at least with DRI3/Present enabled, because the vblank and >>> pageflip ioctls now return -EINVAL while the CRTC is off. However, it >>> looks like fixing that up might not be too bad, so I'm cautiously >>> optimistic for this change. But I'd like some more time for testing and >>> fixing userspace. > > [...] > >> Otoh asking for a vblank event on a dead pipe smells like a userspace bug >> and could result in stuck compositors. Not sure what's best here really. > > Agreed, and we're already careful not to do that with DRI2, just not yet > with DRI3/Present (which isn't in any xf86-video-ati release yet). I've fixed up the DRI3/Present code as well. This patch (with maybe some cleanups to the commit log, in particular I'm not sure the third paragraph should be there) is Acked-and-Tested-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/10] Color Manager Implementation
On 07/13/15 16:07, Daniel Vetter wrote: > On Mon, Jul 13, 2015 at 12:11:08PM +0200, Hans Verkuil wrote: >> On 07/13/2015 11:54 AM, Daniel Vetter wrote: >>> On Mon, Jul 13, 2015 at 11:43:31AM +0200, Hans Verkuil wrote: On 07/13/2015 11:18 AM, Daniel Vetter wrote: > On Mon, Jul 13, 2015 at 10:29:32AM +0200, Hans Verkuil wrote: >> On 06/15/2015 08:53 AM, Daniel Vetter wrote: >>> On Tue, Jun 09, 2015 at 01:50:48PM +0100, Damien Lespiau wrote: On Thu, Jun 04, 2015 at 07:12:31PM +0530, Kausal Malladi wrote: > From: Kausal Malladi > > This patch set adds color manager implementation in drm/i915 layer. > Color Manager is an extension in i915 driver to support color > correction/enhancement. Various Intel platforms support several > color correction capabilities. Color Manager provides abstraction > of these properties and allows a user space UI agent to > correct/enhance the display. So I did a first rough pass on the API itself. The big question that isn't solved at the moment is: do we want to try to do generic KMS properties for pre-LUT + matrix + post-LUT or not. "Generic" has 3 levels: 1/ Generic for all KMS drivers 2/ Generic for i915 supported platfoms 3/ Specific to each platform At this point, I'm quite tempted to say we should give 1/ a shot. We should be able to have pre-LUT + matrix + post-LUT on CRTC objects and guarantee that, when the drivers expose such properties, user space can at least give 8 bits LUT + 3x3 matrix + 8 bits LUT. It may be possible to use the "try" version of the atomic ioctl to explore the space of possibilities from a generic user space to use bigger LUTs as well. A HAL layer (which is already there in some but not all OSes) would still be able to use those generic properties to load "precision optimized" LUTs with some knowledge of the hardware. >>> >>> Yeah, imo 1/ should be doable. For the matrix we should be able to be >>> fully generic with a 16.16 format. For gamma one option would be to have >> >> I know I am late replying, apologies for that. >> >> I've been working on CSC support for V4L2 as well (still work in >> progress) >> and I would like to at least end up with the same low-level fixed point >> format as DRM so we can share matrix/vector calculations. >> >> Based on my experiences I have concerns about the 16.16 format: the >> precision >> is quite low which can be a problem when such values are used in matrix >> multiplications. >> >> In addition, while the precision may be sufficient for 8 bit color >> component >> values, I'm pretty sure it will be insufficient when dealing with 12 or >> 16 bit >> color components. >> >> In earlier versions of my CSC code I used a 12.20 format, but in the >> latest I >> switched to 32.32. This fits nicely in a u64 and it's easy to extract the >> integer and fractional parts. >> >> If this is going to be a generic and future proof API, then my suggestion >> would be to increase the precision of the underlying data type. > > We discussed this a bit more internally and figured it would be nice to > have the same > fixed point for both CSC matrix and LUT/gamma tables. Current consensus > seems to be to go with 8.24 for both. Since LUTs are fairly big I think it > makes sense if we try to be not too wasteful (while still future-proof > ofc). The .24 should have enough precision, but I am worried about the 8: while this works for 8 bit components, you can't use it to represent values > 255, which might be needed (now or in the future) for 10, 12 or 16 bit color components. It's why I ended up with 32.32: it's very generic so usable for other things besides CSC. Note that 8.24 is really 7.24 + one sign bit. So 255 can't be represented in this format. That said, all values I'm working with in my current code are small integers (say between -4 and 4 worst case), so 8.24 would work. But I am not at all confident that this is future proof. My gut feeling is that you need to be able to represent at least the max component value + a sign bit + 7 decimals precision. Which makes 17.24. >>> >>> The idea is to steal from GL and always normalize everything to [0.0, >>> 1.0], irrespective of the source color format. We need that in drm since >>> if you blend together planes with different formats it's completely >>> undefined which one you should pick. 8 bits of precision for values out of >>> range should be enough ;-) >> >> That doesn't really help much, using a [0-1] range just means that you need >> more precision fo
Re: [Intel-gfx] [PATCH v3 06/20] drm/i915: Remove plane_config from struct intel_crtc.
Op 13-07-15 om 18:30 schreef Daniel Stone:
> Hi,
>
> On 13 July 2015 at 15:30, Maarten Lankhorst
> wrote:
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 037a85f1b127..e4d8acc63823 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -15204,7 +15204,9 @@ void intel_modeset_init(struct drm_device *dev)
>> drm_modeset_unlock_all(dev);
>>
>> for_each_intel_crtc(dev, crtc) {
>> - if (!crtc->active)
>> + struct intel_initial_plane_config plane_config;
>> +
>> + if (!crtc->base.state->active)
> Unrelated change from crtc->active to crtc->base.state->active - can
> we do this in one of the later patches?
Probably, but I'm trying to get rid of crtc->active every time I touch a
function.
Eventually this will mean being able to remove it. ;-)
>> + plane_config.fb = NULL;
> memset to 0 instead?
Well for intel_find_initial_plane_obj it's sufficient, but I can just
initialize with plane_config = {}; to keep old behavior.
>> @@ -15713,6 +15716,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> if (ret) {
>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>> to_intel_crtc(c)->pipe);
>> + obj->frontbuffer_bits &=
>> + ~to_intel_plane(c->primary)->frontbuffer_bit;
>> drm_framebuffer_unreference(c->primary->fb);
>> c->primary->fb = NULL;
>> c->primary->crtc = c->primary->state->crtc = NULL;
> Unrelated change?
Unrelated perhaps, but it fixes a warn when pinning fails.
Still I guess a WARN won't hurt in that case, I'll drop it.
> Otherwise:
> Reviewed-by: Daniel Stone
>
> Cheers,
> Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 10/20] drm/i915: Set csc coefficients in intel_sanitize_crtc.
Op 13-07-15 om 18:21 schreef Daniel Stone: > Hi, > > On 13 July 2015 at 15:30, Maarten Lankhorst > wrote: >> This might not have been set during boot, and when we preserve >> the initial mode this can result in a black screen. >> >> Cc: Daniel Stone >> Signed-off-by: Maarten Lankhorst >> --- >> drivers/gpu/drm/i915/intel_display.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c >> b/drivers/gpu/drm/i915/intel_display.c >> index 819a2b551f1f..80e878929bab 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -15251,6 +15251,9 @@ static void intel_sanitize_crtc(struct intel_crtc >> *crtc) >> drm_calc_timestamping_constants(&crtc->base, >> &crtc->base.hwmode); >> update_scanline_offset(crtc); >> drm_crtc_vblank_on(&crtc->base); >> + >> + if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen > 7) >> + intel_set_pipe_csc(&crtc->base); >> } > This is a bit of a weird one - and not your fault. > > intel_set_pipe_csc is currently only called from haswell_crtc_enable, > which is only called for gen >= 9 or HAS_DDI. The (IS_HASWELL || gen > > 7) test covers these devices, plus CHV as well. Does it work on CHV? I think testing for HAS_DDI is enough, works for skylake too. > I'd be more tempted to just call this unconditionally, and stick an > early-exit test in intel_set_pipe_csc instead of at the callsites. But > intel_set_pipe_csc covers gen >= 6, which can never be triggered > through haswell_crtc_enable. So, if we added the early-exit to > set_pipe_csc, we'd also need to remove the previous-gen codepath, or > add calls to set_pipe_csc which didn't previously exist. But CSC is only enabled in update_primary_plane for haswell+, so no point in sanitizing something unused. :-) > But with the test fixed up to be consistent (gen >= 9 || HAS_DDI): > Reviewed-by: Daniel Stone > > Cheers, > Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to stop_rings mode bits
Tim Gore Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Tuesday, July 14, 2015 9:09 AM > To: Gore, Tim > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Wood, Thomas > Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes to > stop_rings > mode bits > > On Mon, Jul 13, 2015 at 04:07:14PM +, Gore, Tim wrote: > > > > > > Tim Gore > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon > > SN3 1RJ > > > > > > > -Original Message- > > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > > > Daniel Vetter > > > Sent: Monday, July 13, 2015 3:59 PM > > > To: Gore, Tim > > > Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org; Wood, Thomas > > > Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow changes > > > to stop_rings mode bits > > > > > > On Mon, Jul 13, 2015 at 09:43:11AM +, Gore, Tim wrote: > > > > > > > > > > > > Tim Gore > > > > Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, > > > > Swindon > > > > SN3 1RJ > > > > > > > > > -Original Message- > > > > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > > > > > Daniel Vetter > > > > > Sent: Monday, July 13, 2015 10:30 AM > > > > > To: Gore, Tim > > > > > Cc: intel-gfx@lists.freedesktop.org; Wood, Thomas > > > > > Subject: Re: [Intel-gfx] [PATCH i-g-t] lib/igt_gt.c : allow > > > > > changes to stop_rings mode bits > > > > > > > > > > On Fri, Jul 10, 2015 at 02:06:06PM +0100, tim.g...@intel.com wrote: > > > > > > From: Tim Gore > > > > > > > > > > > > In function igt_set_stop_rings, the test > > > > > > igt_assert_f(flags == 0 || current == 0, .. > > > > > > > > > > > > will fail if we are trying to force a hang but the > > > > > > STOP_RINGS_ALLOW_BAN or STOP_RINGS_ALLOW_ERROR bit is > set. > > > > > > With the introduction of per ring resets in the driver (in > > > > > > android) these bits do not get cleared to zero when an > > > > > > individual ring is reset. This causes subsequent attempt to > > > > > > cause a ring hang via this function to fail, leading to > > > > > > several igt tests failing (ie gem_reset_stats subtest ban-xxx etc). > > > > > > > > > > Fix tdr to reset these instead? > > > > > -Daniel > > > > > > > > > I could change tdr, but why. When the TDR handles a ring hang and > > > > resets the ring, why would it modify the flag that defines if the > > > > driver should ban a frequently hanging context? If we get rid of > > > > the stop_rings interface, as Chris Wilson suggested, we would > > > > still need to keep the STOP_RING_ALLOW_BAN/ALLOW_ERRORS bits in > > > > debugfs, but > > > you > > > > would not expect to have to re-write these bits each time there is > > > > a ring > > > reset. > > > > > > The fix current hang recover code to no reset this, add some grace > > > period, then push this patch to igt. We don't have full-blown abi > > > guarantees for debugfs/igt stuff, but I want at least a few months > > > (really last released > > > kernel&igt) of backwards/forward compatibility. And inconsistent > > > behaviour isn't great imo. > > > -Daniel > > > > Sorry Daniel, I didn't really follow that. > > I didn't want a gpu reset to clear the ALLOW_BAN bit, since this will > > defeat the point of this bit, except perhaps in test situations where > > you can keep setting it each time you deliberately cause a hang. It > > seems like the ALLOW_BAN bit has uses in real world situations, although I > don't know it anyone uses it. > > ALLOW_BAN is only for testing. It's meant to re-enable auto-banning > because we disable that by default for automated tests. This is all meant to > be used together with the stop_rings stuff only. > > > Would you be more comfortable with > > Igt_assert_f ( ( flags ==0 ) || > > (( current & STOP_RING_ALL) ==0) && ((current ^ flags) & ~ > > STOP_RING_ALL == 0 ) ) > > > > So the either the new flags must be 0 (currently allowed) or the > > existing flags must indicate that all hangs are cleared (0 except > > possibly the mode bits) AND the mode bits you are writing are the same as > the current values. ?? > > Iirc gem_reset_stats uses stop_rings only to supress gpu reset warnings in > dmesg (since they're expected) and not for the actual stop_rings logic. It > sets > stop_rings after submitting the hanging batch, but before that one is > detected as hung. We could just add another bit to stop_rings for that case. > > What I still don't understand is why tdr can't just keep on properly resetting > stop_rings. It'll break tons of existing tests, and I don't understand the > upside. stop_rings has become quasi-abi (that's why the ALLOW bits have > such funky semantics, it's for backwards compat), if you need to change it > you need to extend it, not break it. > -Daniel > -- OK, I hadn't spotted that it was a debug only flag. In that c
[Intel-gfx] [PATCH 1/2] drm/i915: Remove unused compat32 code
Totatlly forgotten that we have these when nuking all the UMS code.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_ioc32.c | 125 --
1 file changed, 125 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_ioc32.c
b/drivers/gpu/drm/i915/i915_ioc32.c
index 176de6322e4d..6eec2221b44e 100644
--- a/drivers/gpu/drm/i915/i915_ioc32.c
+++ b/drivers/gpu/drm/i915/i915_ioc32.c
@@ -35,98 +35,6 @@
#include
#include "i915_drv.h"
-typedef struct _drm_i915_batchbuffer32 {
- int start; /* agp offset */
- int used; /* nr bytes in use */
- int DR1;/* hw flags for GFX_OP_DRAWRECT_INFO */
- int DR4;/* window origin for GFX_OP_DRAWRECT_INFO */
- int num_cliprects; /* mulitpass with multiple cliprects? */
- u32 cliprects; /* pointer to userspace cliprects */
-} drm_i915_batchbuffer32_t;
-
-static int compat_i915_batchbuffer(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- drm_i915_batchbuffer32_t batchbuffer32;
- drm_i915_batchbuffer_t __user *batchbuffer;
-
- if (copy_from_user
- (&batchbuffer32, (void __user *)arg, sizeof(batchbuffer32)))
- return -EFAULT;
-
- batchbuffer = compat_alloc_user_space(sizeof(*batchbuffer));
- if (!access_ok(VERIFY_WRITE, batchbuffer, sizeof(*batchbuffer))
- || __put_user(batchbuffer32.start, &batchbuffer->start)
- || __put_user(batchbuffer32.used, &batchbuffer->used)
- || __put_user(batchbuffer32.DR1, &batchbuffer->DR1)
- || __put_user(batchbuffer32.DR4, &batchbuffer->DR4)
- || __put_user(batchbuffer32.num_cliprects,
- &batchbuffer->num_cliprects)
- || __put_user((int __user *)(unsigned long)batchbuffer32.cliprects,
- &batchbuffer->cliprects))
- return -EFAULT;
-
- return drm_ioctl(file, DRM_IOCTL_I915_BATCHBUFFER,
-(unsigned long)batchbuffer);
-}
-
-typedef struct _drm_i915_cmdbuffer32 {
- u32 buf;/* pointer to userspace command buffer */
- int sz; /* nr bytes in buf */
- int DR1;/* hw flags for GFX_OP_DRAWRECT_INFO */
- int DR4;/* window origin for GFX_OP_DRAWRECT_INFO */
- int num_cliprects; /* mulitpass with multiple cliprects? */
- u32 cliprects; /* pointer to userspace cliprects */
-} drm_i915_cmdbuffer32_t;
-
-static int compat_i915_cmdbuffer(struct file *file, unsigned int cmd,
-unsigned long arg)
-{
- drm_i915_cmdbuffer32_t cmdbuffer32;
- drm_i915_cmdbuffer_t __user *cmdbuffer;
-
- if (copy_from_user
- (&cmdbuffer32, (void __user *)arg, sizeof(cmdbuffer32)))
- return -EFAULT;
-
- cmdbuffer = compat_alloc_user_space(sizeof(*cmdbuffer));
- if (!access_ok(VERIFY_WRITE, cmdbuffer, sizeof(*cmdbuffer))
- || __put_user((int __user *)(unsigned long)cmdbuffer32.buf,
- &cmdbuffer->buf)
- || __put_user(cmdbuffer32.sz, &cmdbuffer->sz)
- || __put_user(cmdbuffer32.DR1, &cmdbuffer->DR1)
- || __put_user(cmdbuffer32.DR4, &cmdbuffer->DR4)
- || __put_user(cmdbuffer32.num_cliprects, &cmdbuffer->num_cliprects)
- || __put_user((int __user *)(unsigned long)cmdbuffer32.cliprects,
- &cmdbuffer->cliprects))
- return -EFAULT;
-
- return drm_ioctl(file, DRM_IOCTL_I915_CMDBUFFER,
-(unsigned long)cmdbuffer);
-}
-
-typedef struct drm_i915_irq_emit32 {
- u32 irq_seq;
-} drm_i915_irq_emit32_t;
-
-static int compat_i915_irq_emit(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- drm_i915_irq_emit32_t req32;
- drm_i915_irq_emit_t __user *request;
-
- if (copy_from_user(&req32, (void __user *)arg, sizeof(req32)))
- return -EFAULT;
-
- request = compat_alloc_user_space(sizeof(*request));
- if (!access_ok(VERIFY_WRITE, request, sizeof(*request))
- || __put_user((int __user *)(unsigned long)req32.irq_seq,
- &request->irq_seq))
- return -EFAULT;
-
- return drm_ioctl(file, DRM_IOCTL_I915_IRQ_EMIT,
-(unsigned long)request);
-}
typedef struct drm_i915_getparam32 {
int param;
u32 value;
@@ -152,41 +60,8 @@ static int compat_i915_getparam(struct file *file, unsigned
int cmd,
(unsigned long)request);
}
-typedef struct drm_i915_mem_alloc32 {
- int region;
- int alignment;
- int size;
- u32 region_offset; /* offset from start of fb or agp */
-} drm_i915_mem_alloc32_t;
-
-static int compat_i915_alloc(struct file *file, unsigned int cmd,
-
[Intel-gfx] [PATCH 2/2] drm/i915: Use expcitly fixed type in compat32 structs
I was confused shortly whether the compat was needed for the int,
until I noticed the pointer in the original.
Also remove typedef.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_ioc32.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_ioc32.c
b/drivers/gpu/drm/i915/i915_ioc32.c
index 6eec2221b44e..ff5dc1d0d864 100644
--- a/drivers/gpu/drm/i915/i915_ioc32.c
+++ b/drivers/gpu/drm/i915/i915_ioc32.c
@@ -35,15 +35,15 @@
#include
#include "i915_drv.h"
-typedef struct drm_i915_getparam32 {
- int param;
+struct drm_i915_getparam32 {
+ s32 param;
u32 value;
-} drm_i915_getparam32_t;
+};
static int compat_i915_getparam(struct file *file, unsigned int cmd,
unsigned long arg)
{
- drm_i915_getparam32_t req32;
+ struct drm_i915_getparam32 req32;
drm_i915_getparam_t __user *request;
if (copy_from_user(&req32, (void __user *)arg, sizeof(req32)))
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect
On Tue, Jul 14, 2015 at 08:09:21AM +, Sharma, Shashank wrote: > Please apply this patch series along with the Interrupt handling fix patch > Sonika shared. > With these 4 patches applied, we should not see any problems with HPDs (Until > the HW is broken). Ok, that explains things. If you have depencies please include them all in your patch series and don't put it all over the mailing lists. I won't be able to keep track of everything. I was simply confused about why it should suddenly work without that fix. Can you please resend the entire series with all ingredients required? And enabled on all platforms for maximum testing? Thanks, Daniel > On a similar note, the corresponding chrome team applied the live status > check patch, along with others, and they > are not seeing the problems with HPD, hence they are also interested in this > patch series. > > Please let us know if you observe something else, we would love to dig > further. > But as we previously mentioned, this patch series is available and running > across various MCG Gen7 devices, and available with > Gen8 PV production branches also, and there the hotplug stability is pretty > good. > > Regards > Shashank > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter > Sent: Tuesday, July 14, 2015 1:29 PM > To: Sharma, Shashank > Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; > intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from > detect > > On Tue, Jul 14, 2015 at 03:48:36AM +, Sharma, Shashank wrote: > > Hi Daniel, Chris > > > > Gen7 and Gen8 platforms have a different live status register (0x61114) and > > we need not to rely on ISR on that. > > As Sonika mentioned, We have been using this register for our commercial > > releases, and found them reliable across a wide range of monitors. > > > > On the other hand, Bsepc clearly mentions, to check the live status before > > even try to read EDID. > > The current DRM nightly code doesn't do that, and we have received few > > errors from Gen7 Chromebooks where you can still read valid EDID on HDMI > > hot-unplug. > > > > So I think this patch and solution is ready, and it should go in. > > I have a gen7 machine here which is currently (and it's somewhat random) > broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix > for the hpd handling - or did I miss it? > -Daniel > > > > > Regards > > Shashank > > -Original Message- > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > > Daniel Vetter > > Sent: Monday, July 13, 2015 8:27 PM > > To: Jindal, Sonika > > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > > from detect > > > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > > >>During init_connector set the edid, then edid will be set/unset > > > >>only during hotplug. For the sake of older platforms where HPD is > > > >>not stable, let edid read happen from detect as well only if it is > > > >>forced to do so. > > > >> > > > >>v2: Removing the 'force' check, instead let detect call read the > > > >>edid for platforms older than gen 7 (Shashank) > > > > > > > >That's enough worse. We now have a random gen check with no > > > >rationale for why you suddenly believe all manufacturers have fixed > > > >their wiring. > > > >There is no reason to believe that gen7 suddenly works is there? If > > > >there is, why don't you mention it? > > > >-Chris > > > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > > For CHV/VLV, already the live status check is stable enough to > > > determine if we can read edid or not. In VPG production branches we > > > have this patch already available and it was tested with variety of > > > monitors extensively. So we now read the edid only during init and during > > > hotplug. > > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > > patch makes HPD stable enough. > > > So, we can rely on the edid read from init_connector instead of > > > reading edid on every detect call. > > > > Again, not going to take this with random gen checks. I want your fix for > > handling hpd on other platforms, then roll this out everywhere. > > -Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/10] Color Manager Implementation
On Tue, Jul 14, 2015 at 10:17:09AM +0200, Hans Verkuil wrote: > On 07/13/15 16:07, Daniel Vetter wrote: > > On Mon, Jul 13, 2015 at 12:11:08PM +0200, Hans Verkuil wrote: > >> On 07/13/2015 11:54 AM, Daniel Vetter wrote: > >>> On Mon, Jul 13, 2015 at 11:43:31AM +0200, Hans Verkuil wrote: > On 07/13/2015 11:18 AM, Daniel Vetter wrote: > > On Mon, Jul 13, 2015 at 10:29:32AM +0200, Hans Verkuil wrote: > >> On 06/15/2015 08:53 AM, Daniel Vetter wrote: > >>> On Tue, Jun 09, 2015 at 01:50:48PM +0100, Damien Lespiau wrote: > On Thu, Jun 04, 2015 at 07:12:31PM +0530, Kausal Malladi wrote: > > From: Kausal Malladi > > > > This patch set adds color manager implementation in drm/i915 layer. > > Color Manager is an extension in i915 driver to support color > > correction/enhancement. Various Intel platforms support several > > color correction capabilities. Color Manager provides abstraction > > of these properties and allows a user space UI agent to > > correct/enhance the display. > > So I did a first rough pass on the API itself. The big question that > isn't solved at the moment is: do we want to try to do generic KMS > properties for pre-LUT + matrix + post-LUT or not. "Generic" has 3 > levels: > > 1/ Generic for all KMS drivers > 2/ Generic for i915 supported platfoms > 3/ Specific to each platform > > At this point, I'm quite tempted to say we should give 1/ a shot. We > should be able to have pre-LUT + matrix + post-LUT on CRTC objects > and > guarantee that, when the drivers expose such properties, user space > can > at least give 8 bits LUT + 3x3 matrix + 8 bits LUT. > > It may be possible to use the "try" version of the atomic ioctl to > explore the space of possibilities from a generic user space to use > bigger LUTs as well. A HAL layer (which is already there in some but > not > all OSes) would still be able to use those generic properties to load > "precision optimized" LUTs with some knowledge of the hardware. > >>> > >>> Yeah, imo 1/ should be doable. For the matrix we should be able to be > >>> fully generic with a 16.16 format. For gamma one option would be to > >>> have > >> > >> I know I am late replying, apologies for that. > >> > >> I've been working on CSC support for V4L2 as well (still work in > >> progress) > >> and I would like to at least end up with the same low-level fixed point > >> format as DRM so we can share matrix/vector calculations. > >> > >> Based on my experiences I have concerns about the 16.16 format: the > >> precision > >> is quite low which can be a problem when such values are used in matrix > >> multiplications. > >> > >> In addition, while the precision may be sufficient for 8 bit color > >> component > >> values, I'm pretty sure it will be insufficient when dealing with 12 > >> or 16 bit > >> color components. > >> > >> In earlier versions of my CSC code I used a 12.20 format, but in the > >> latest I > >> switched to 32.32. This fits nicely in a u64 and it's easy to extract > >> the > >> integer and fractional parts. > >> > >> If this is going to be a generic and future proof API, then my > >> suggestion > >> would be to increase the precision of the underlying data type. > > > > We discussed this a bit more internally and figured it would be nice to > > have the same > > fixed point for both CSC matrix and LUT/gamma tables. Current consensus > > seems to be to go with 8.24 for both. Since LUTs are fairly big I think > > it > > makes sense if we try to be not too wasteful (while still future-proof > > ofc). > > The .24 should have enough precision, but I am worried about the 8: while > this works for 8 bit components, you can't use it to represent values > > 255, which might be needed (now or in the future) for 10, 12 or 16 bit > color components. > > It's why I ended up with 32.32: it's very generic so usable for other > things besides CSC. > > Note that 8.24 is really 7.24 + one sign bit. So 255 can't be represented > in this format. > > That said, all values I'm working with in my current code are small > integers > (say between -4 and 4 worst case), so 8.24 would work. But I am not at > all > confident that this is future proof. My gut feeling is that you need to > be > able to represent at least the max component value + a sign bit + 7 > decimals > precision. Which makes 17.24. > >>> > >>> The idea is to steal from GL and always normalize everything to [0.0, > >>> 1.0], irrespective of t
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove unused compat32 code
On Tue, Jul 14, 2015 at 10:59:30AM +0200, Daniel Vetter wrote: > Totatlly forgotten that we have these when nuking all the UMS code. > > Signed-off-by: Daniel Vetter All deleted UMS stubs, 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
Re: [Intel-gfx] [RFC 1/1] drm/i915 : Wait until SYSTEM_RUNNING before loading CSR firmware
On Mon, Jul 13, 2015 at 09:36:45AM -0700, jay.p.pa...@intel.com wrote:
> From: Jay Patel
>
> NOTE: This is an interim solution which is targeted towards
> Chrome OS/Android to be used until a long term solution is available.
>
> In this patch, request_firmware() is called in a worker thread
> which initially waits for file system to be initialized and then
> attempts to load the firmware.
Aside: I posted a bunch of proof-of-principle patches to clean up dmc
loading and convert over to using an explicit workqueue. They're being
tested/made-to-actually-work right now I think.
> "request_firmware_nowait()" is also using an asynchronous thread
> running concurrently with the rest of the system initialization.
> However, it tries to load firmware only once without checking the
> sytem status and fails most of the time.
>
> Change-Id: I2cb16a768e54a85f48a6682d9690b4c8af844668
> Signed-off-by: Jay Patel
> ---
> drivers/gpu/drm/i915/i915_drv.c | 2 ++
> drivers/gpu/drm/i915/intel_csr.c | 58
>
> 2 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8c8407d..eb6f7e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -559,6 +559,7 @@ void intel_hpd_cancel_work(struct drm_i915_private
> *dev_priv)
> void i915_firmware_load_error_print(const char *fw_path, int err)
> {
> DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> + DRM_ERROR("The firmware file may be missing\n");
>
> /*
>* If the reason is not known assume -ENOENT since that's the most
> @@ -574,6 +575,7 @@ void i915_firmware_load_error_print(const char *fw_path,
> int err)
> "The driver is built-in, so to load the firmware you need to\n"
> "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> "in your initrd/initramfs image.\n");
> +
> }
>
> static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_csr.c
> b/drivers/gpu/drm/i915/intel_csr.c
> index 9311cdd..8d1f08c 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -349,7 +349,7 @@ static void finish_csr_load(const struct firmware *fw,
> void *context)
> /* load csr program during system boot, as needed for DC states */
> intel_csr_load_program(dev);
> fw_loaded = true;
> -
> + DRM_INFO("CSR Firmware Loaded\n");
> out:
> if (fw_loaded)
> intel_runtime_pm_put(dev_priv);
> @@ -359,11 +359,46 @@ out:
> release_firmware(fw);
> }
>
> +struct csr_firmware_work {
> + struct work_struct work;
> + struct module *module;
> + struct drm_device *dev;
> +};
> +
> +/* csr_firmware_work_func() - thread function for loading the firmware*/
> +static void csr_firmware_work_func(struct work_struct *work)
> +{
> + const struct firmware *fw;
> + const struct csr_firmware_work *fw_work = container_of(work, struct
> csr_firmware_work, work);
> + int ret;
> + struct drm_device *dev = fw_work->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_csr *csr = &dev_priv->csr;
> +
> + /* Wait until root filesystem is loaded in case the firmware
> + * is not built-in but in /lib/firmware */
> + while(system_state != SYSTEM_RUNNING){
> + msleep(500);
> + }
Yeah, not going to merge that right now until we've had a decent
discussion with Greg KH (since imo his stance of every driver creating
it's own retry loop just doesn't work, especially not with gfx where init
is hairy and you just don't want to retry without end).
Aside: Another solution might be the wait_for_rootfs from
http://www.gossamer-threads.com/lists/linux/kernel/2010793
But if Greg insists that each driver needs to solve this themselves then
I'll pull something like this into upstream, but probably with a Kconfig
option to disable it for normal linux userspace. The other option would
be to use CONFIG_FW_LOADER_USERSPACE_FALLBACK udev helper. That might be
an option for intel android, but it sounds like not something cros wants
to do. Therefore
Acked-by: Daniel Vetter
Also adding Greg so he knows what's happening here.
-Daniel
> +
> + ret = request_firmware(&fw, csr->fw_path, &dev_priv->dev->pdev->dev);
> +if (ret) {
> + i915_firmware_load_error_print(csr->fw_path, ret);
> + intel_csr_load_status_set(dev_priv, FW_FAILED);
> + } else {
> + finish_csr_load(fw, dev_priv);
> + put_device(&dev_priv->dev->pdev->dev);
> + module_put(fw_work->module);
> + }
> +
> + kfree(fw_work);
> +}
> +
> void intel_csr_ucode_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_csr *csr = &dev_priv->csr;
> - int ret;
> + struct csr_fir
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use expcitly fixed type in compat32 structs
On Tue, Jul 14, 2015 at 10:59:31AM +0200, Daniel Vetter wrote: > I was confused shortly whether the compat was needed for the int, > until I noticed the pointer in the original. > > Also remove typedef. > > Signed-off-by: Daniel Vetter Oh, why an int? But you should improve the uapi definition as well. And add a (short) paragraph before it to explain exactly why we should never ever do this again. -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 3/6] drm/i915/skl: Restrict the ring frequency table programming to SKL
On Mon, Jun 29, 2015 at 02:50:21PM +0530, akash.g...@intel.com wrote:
> From: Akash Goel
>
> Ring frequency table programming is not required on BXT. Added separate
> checks to enable the programming only for SKL & skip for BXT.
>
> v2: Removed the BXT check from gen6_update_ring_freq function
>
> Issue: VIZ-5144
> Signed-off-by: Akash Goel
Queued for -next with Rodrigo's r-b from archives (somehow that mail
didn't make it to me), thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_pm.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 89c1b73..daca7e7 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5811,7 +5811,8 @@ static void intel_gen6_powersave_work(struct
> work_struct *work)
> } else if (INTEL_INFO(dev)->gen >= 9) {
> gen9_enable_rc6(dev);
> gen9_enable_rps(dev);
> - __gen6_update_ring_freq(dev);
> + if (IS_SKYLAKE(dev))
> + __gen6_update_ring_freq(dev);
> } else if (IS_BROADWELL(dev)) {
> gen8_enable_rps(dev);
> __gen6_update_ring_freq(dev);
> --
> 1.9.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
On 13/07/15 14:16, Mika Kuoppala wrote:
Arun Siluvery writes:
In Indirect context w/a batch buffer,
+WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).
Cc: Robert Beckett
Cc: Imre Deak
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_lrc.c| 9 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 58247f0..61ed92b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
*ring,
struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+ /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
+ if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
+ (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
+ wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
+ wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
+ wa_ctx_emit(batch,
_MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
+ wa_ctx_emit(batch, MI_NOOP);
+ }
+
/* WaDisableCtxRestoreArbitration:skl,bxt */
if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
(IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af7c12e..66dde7f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
/*
WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
GEN9_RHWO_OPTIMIZATION_DISABLE);
- WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
- DISABLE_PIXEL_MASK_CAMMING);
+ /*
+* WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be
set,
+* but that register is write only hence it is set
+* in per ctx batch buffer
Why the need to move to per ctx bb? Why the write would not stick
with this? Is the rationale that we get fails with the gem_workarounds?
If so, perhaps we need a write_only marker for workaround list?
-Mika
Surely it's the fact that it's a context-saved register that matters,
rather than it being write-only (generally we treat masked registers as
write-only anyway, since the masking means that you can set or clear a
bit without having to read the other bits first).
Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context
register, so it can be set directly rather than having to be done in a
batchbuffer.
.Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/10] Color Manager Implementation
On 07/14/15 11:11, Daniel Vetter wrote: > On Tue, Jul 14, 2015 at 10:17:09AM +0200, Hans Verkuil wrote: >> On 07/13/15 16:07, Daniel Vetter wrote: >>> On Mon, Jul 13, 2015 at 12:11:08PM +0200, Hans Verkuil wrote: On 07/13/2015 11:54 AM, Daniel Vetter wrote: > On Mon, Jul 13, 2015 at 11:43:31AM +0200, Hans Verkuil wrote: >> On 07/13/2015 11:18 AM, Daniel Vetter wrote: >>> On Mon, Jul 13, 2015 at 10:29:32AM +0200, Hans Verkuil wrote: On 06/15/2015 08:53 AM, Daniel Vetter wrote: > On Tue, Jun 09, 2015 at 01:50:48PM +0100, Damien Lespiau wrote: >> On Thu, Jun 04, 2015 at 07:12:31PM +0530, Kausal Malladi wrote: >>> From: Kausal Malladi >>> >>> This patch set adds color manager implementation in drm/i915 layer. >>> Color Manager is an extension in i915 driver to support color >>> correction/enhancement. Various Intel platforms support several >>> color correction capabilities. Color Manager provides abstraction >>> of these properties and allows a user space UI agent to >>> correct/enhance the display. >> >> So I did a first rough pass on the API itself. The big question that >> isn't solved at the moment is: do we want to try to do generic KMS >> properties for pre-LUT + matrix + post-LUT or not. "Generic" has 3 >> levels: >> >> 1/ Generic for all KMS drivers >> 2/ Generic for i915 supported platfoms >> 3/ Specific to each platform >> >> At this point, I'm quite tempted to say we should give 1/ a shot. We >> should be able to have pre-LUT + matrix + post-LUT on CRTC objects >> and >> guarantee that, when the drivers expose such properties, user space >> can >> at least give 8 bits LUT + 3x3 matrix + 8 bits LUT. >> >> It may be possible to use the "try" version of the atomic ioctl to >> explore the space of possibilities from a generic user space to use >> bigger LUTs as well. A HAL layer (which is already there in some but >> not >> all OSes) would still be able to use those generic properties to load >> "precision optimized" LUTs with some knowledge of the hardware. > > Yeah, imo 1/ should be doable. For the matrix we should be able to be > fully generic with a 16.16 format. For gamma one option would be to > have I know I am late replying, apologies for that. I've been working on CSC support for V4L2 as well (still work in progress) and I would like to at least end up with the same low-level fixed point format as DRM so we can share matrix/vector calculations. Based on my experiences I have concerns about the 16.16 format: the precision is quite low which can be a problem when such values are used in matrix multiplications. In addition, while the precision may be sufficient for 8 bit color component values, I'm pretty sure it will be insufficient when dealing with 12 or 16 bit color components. In earlier versions of my CSC code I used a 12.20 format, but in the latest I switched to 32.32. This fits nicely in a u64 and it's easy to extract the integer and fractional parts. If this is going to be a generic and future proof API, then my suggestion would be to increase the precision of the underlying data type. >>> >>> We discussed this a bit more internally and figured it would be nice to >>> have the same >>> fixed point for both CSC matrix and LUT/gamma tables. Current consensus >>> seems to be to go with 8.24 for both. Since LUTs are fairly big I think >>> it >>> makes sense if we try to be not too wasteful (while still future-proof >>> ofc). >> >> The .24 should have enough precision, but I am worried about the 8: while >> this works for 8 bit components, you can't use it to represent values >>> 255, which might be needed (now or in the future) for 10, 12 or 16 bit >> color components. >> >> It's why I ended up with 32.32: it's very generic so usable for other >> things besides CSC. >> >> Note that 8.24 is really 7.24 + one sign bit. So 255 can't be represented >> in this format. >> >> That said, all values I'm working with in my current code are small >> integers >> (say between -4 and 4 worst case), so 8.24 would work. But I am not at >> all >> confident that this is future proof. My gut feeling is that you need to >> be >> able to represent at least the max component value + a sign bit + 7 >> decimals >> precision. Which makes 17.24. > > The idea is to steal from GL and always normalize everyt
Re: [Intel-gfx] [PATCH v3 09/20] drm/i915: Fill in more crtc state, v3.
On Mon, Jul 13, 2015 at 04:30:22PM +0200, Maarten Lankhorst wrote:
> There is a small memory leak in intel_modeset_readout_hw_state,
> plug it.
This should be a separate patch I think since it seems unrelated to the
other changes. And I think less mystery in the commit message helps, e.g.
"We leak crtc state references in the hw state readout, fix this." instead
of talking about an onimous small leak ;-)
-Daniel
>
> intel_sanitize_crtc should set a null mode when disabling the crtc,
> this updates crtc_state->enable too.
>
> intel_sanitize_crtc also needs to update the vblank timestamps before
> enabling vblank to make it work right.
>
> Changes since v1:
> - Moved after reworking primary plane and updating mode members.
> - Force a modeset calculation by changing mode->type from what the
> final mode will be.
> Changes since v2:
> - Do not fill out mode, this should only be done for supporting fastboot.
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index d37f6a93b094..819a2b551f1f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15248,6 +15248,7 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> /* restore vblank interrupts to correct state */
> drm_crtc_vblank_reset(&crtc->base);
> if (crtc->active) {
> + drm_calc_timestamping_constants(&crtc->base,
> &crtc->base.hwmode);
> update_scanline_offset(crtc);
> drm_crtc_vblank_on(&crtc->base);
> }
> @@ -15300,7 +15301,7 @@ static void intel_sanitize_crtc(struct intel_crtc
> *crtc)
> crtc->base.state->enable ? "enabled" : "disabled",
> crtc->active ? "enabled" : "disabled");
>
> - crtc->base.state->enable = crtc->active;
> + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) <
> 0);
> crtc->base.state->active = crtc->active;
> crtc->base.enabled = crtc->active;
>
> @@ -15450,6 +15451,7 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
> int i;
>
> for_each_intel_crtc(dev, crtc) {
> + __drm_atomic_helper_crtc_destroy_state(&crtc->base,
> crtc->base.state);
> memset(crtc->config, 0, sizeof(*crtc->config));
> crtc->config->base.crtc = &crtc->base;
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 04/20] drm/i915: Allow fuzzy matching in pipe_config_compare, v2.
On Mon, Jul 13, 2015 at 04:30:17PM +0200, Maarten Lankhorst wrote:
> Instead of doing ad-hoc checks we already have a way of checking
> if the state is compatible or not. Use this to force a modeset.
>
> Only during modesets, or with PIPE_CONFIG_QUIRK_INHERITED_MODE
> we should check if a full modeset is really needed.
>
> Fastboot will allow the adjust parameter to ignore some stuff
> too, and it will fix up differences in state that are ignored
> by the compare function.
>
> Changes since v1:
> - Increase the value of the lowest m/n to prevent truncation.
> - Dump pipe config when fastboot's used, without a modeset.
Needs to mention that the dp M/N comparison helper grew the "exact"
parameter.
-Daniel
>
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/i915/intel_display.c | 218
> +--
> 1 file changed, 157 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6eed925f3300..c3a3d108 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12290,19 +12290,6 @@ encoder_retry:
> DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
> base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>
> - /* Check if we need to force a modeset */
> - if (pipe_config->has_audio !=
> - to_intel_crtc_state(crtc->state)->has_audio) {
> - pipe_config->base.mode_changed = true;
> - ret = drm_atomic_add_affected_planes(state, crtc);
> - }
> -
> - /*
> - * Note we have an issue here with infoframes: current code
> - * only updates them on the full mode set path per hw
> - * requirements. So here we should be checking for any
> - * required changes and forcing a mode set.
> - */
> fail:
> return ret;
> }
> @@ -12406,27 +12393,133 @@ static bool intel_fuzzy_clock_check(int clock1,
> int clock2)
> base.head) \
> if (mask & (1 <<(intel_crtc)->pipe))
>
> +
> +static bool
> +intel_compare_m_n(unsigned int m, unsigned int n,
> + unsigned int m2, unsigned int n2,
> + bool exact)
> +{
> + if (m == m2 && n == n2)
> + return true;
> +
> + if (exact || !m || !n || !m2 || !n2)
> + return false;
> +
> + BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
> +
> + if (m > m2) {
> + while (m > m2) {
> + m2 <<= 1;
> + n2 <<= 1;
> + }
> + } else if (m < m2) {
> + while (m < m2) {
> + m <<= 1;
> + n <<= 1;
> + }
> + }
> +
> + return m == m2 && n == n2;
> +}
> +
> +static bool
> +intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> +struct intel_link_m_n *m2_n2,
> +bool adjust)
> +{
> + if (m_n->tu == m2_n2->tu &&
> + intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
> + m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
> + intel_compare_m_n(m_n->link_m, m_n->link_n,
> + m2_n2->link_m, m2_n2->link_n, !adjust)) {
> + if (adjust)
> + *m2_n2 = *m_n;
> +
> + return true;
> + }
> +
> + return false;
> +}
> +
> static bool
> intel_pipe_config_compare(struct drm_device *dev,
> struct intel_crtc_state *current_config,
> - struct intel_crtc_state *pipe_config)
> + struct intel_crtc_state *pipe_config,
> + bool adjust)
> {
> + bool ret = true;
> +
> +#define INTEL_ERR_OR_DBG_KMS(fmt, ...) \
> + do { \
> + if (!adjust) \
> + DRM_ERROR(fmt, ##__VA_ARGS__); \
> + else \
> + DRM_DEBUG_KMS(fmt, ##__VA_ARGS__); \
> + } while (0)
> +
> #define PIPE_CONF_CHECK_X(name) \
> if (current_config->name != pipe_config->name) { \
> - DRM_ERROR("mismatch in " #name " " \
> + INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
> "(expected 0x%08x, found 0x%08x)\n", \
> current_config->name, \
> pipe_config->name); \
> - return false; \
> + ret = false; \
> }
>
> #define PIPE_CONF_CHECK_I(name) \
> if (current_config->name != pipe_config->name) { \
> - DRM_ERROR("mismatch in " #name " " \
> + INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
> "(expected %i, found %i)\n", \
> current_config->name, \
> pipe_config->name); \
> - return false; \
> + ret = false; \
> + }
> +
> +#define PIPE_CONF_CHECK_M_N(name) \
> + if (!intel_compare_link_m_n(¤t_config->name
Re: [Intel-gfx] [PATCH v3 08/20] drm/i915: fill in more mode members
On Mon, Jul 13, 2015 at 04:30:21PM +0200, Maarten Lankhorst wrote: > Signed-off-by: Maarten Lankhorst Needs a summary (or just pasting relevant parts of our mail threads) about double-modesets, ways to avoid them and why exactly we ended up opting for this solution here. Especially please highlight the TYPE_DRIVER trick. Thanks, Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 118187dc76be..d37f6a93b094 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -7754,9 +7754,14 @@ void intel_mode_from_pipe_config(struct > drm_display_mode *mode, > mode->vsync_end = pipe_config->base.adjusted_mode.crtc_vsync_end; > > mode->flags = pipe_config->base.adjusted_mode.flags; > + mode->type = DRM_MODE_TYPE_DRIVER; > > mode->clock = pipe_config->base.adjusted_mode.crtc_clock; > mode->flags |= pipe_config->base.adjusted_mode.flags; > + > + mode->hsync = drm_mode_hsync(mode); > + mode->vrefresh = drm_mode_vrefresh(mode); > + drm_mode_set_name(mode); > } > > static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) > -- > 2.1.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 07/20] drm/i915: Rework plane readout.
On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote:
> All non-primary planes get disabled during hw readout,
> this reduces complexity and means not having to do some plane
> visibility checks during the first commit.
>
> Signed-off-by: Maarten Lankhorst
I still think calling readout_plane_state from the hw state readout code
is the wrong approach. Instead we should consolidate all the plane
readout code exclusively into a new intel_plane_readout_hw_state helper
which is called only from driver load paths. That should also contain the
fb/gem_bo reconstruction loop.
For the other 2 users of this code (lid notifiery and resume) we should
just force an unconditional plane restore by setting
crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work,
once atomic has settled a bit more.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 7 ---
> drivers/gpu/drm/i915/intel_display.c | 86
>
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 3 files changed, 8 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index b92b8581efc2..dcf4fb458649 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -98,13 +98,6 @@ int intel_atomic_check(struct drm_device *dev,
> return -EINVAL;
> }
>
> - if (crtc_state &&
> - crtc_state->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> - ret = drm_atomic_add_affected_planes(state,
> &nuclear_crtc->base);
> - if (ret)
> - return ret;
> - }
> -
> ret = drm_atomic_helper_check_planes(dev, state);
> if (ret)
> return ret;
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e4d8acc63823..118187dc76be 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11783,34 +11783,6 @@ static bool check_encoder_cloning(struct
> drm_atomic_state *state,
> return true;
> }
>
> -static void intel_crtc_check_initial_planes(struct drm_crtc *crtc,
> - struct drm_crtc_state *crtc_state)
> -{
> - struct intel_crtc_state *pipe_config =
> - to_intel_crtc_state(crtc_state);
> - struct drm_plane *p;
> - unsigned visible_mask = 0;
> -
> - drm_for_each_plane_mask(p, crtc->dev, crtc_state->plane_mask) {
> - struct drm_plane_state *plane_state =
> - drm_atomic_get_existing_plane_state(crtc_state->state,
> p);
> -
> - if (WARN_ON(!plane_state))
> - continue;
> -
> - if (!plane_state->fb)
> - crtc_state->plane_mask &=
> - ~(1 << drm_plane_index(p));
> - else if (to_intel_plane_state(plane_state)->visible)
> - visible_mask |= 1 << drm_plane_index(p);
> - }
> -
> - if (!visible_mask)
> - return;
> -
> - pipe_config->quirks &= ~PIPE_CONFIG_QUIRK_INITIAL_PLANES;
> -}
> -
> static int intel_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_crtc_state *crtc_state)
> {
> @@ -11832,10 +11804,6 @@ static int intel_crtc_atomic_check(struct drm_crtc
> *crtc,
> "[CRTC:%i] mismatch between state->active(%i) and
> crtc->active(%i)\n",
> idx, crtc->state->active, intel_crtc->active);
>
> - /* plane mask is fixed up after all initial planes are calculated */
> - if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES)
> - intel_crtc_check_initial_planes(crtc, crtc_state);
> -
> if (mode_changed && !crtc_state->active)
> intel_crtc->atomic.update_wm_post = true;
>
> @@ -13188,19 +13156,6 @@ intel_modeset_compute_config(struct drm_atomic_state
> *state)
> continue;
> }
>
> - if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES) {
> - ret = drm_atomic_add_affected_planes(state, crtc);
> - if (ret)
> - return ret;
> -
> - /*
> - * We ought to handle i915.fastboot here.
> - * If no modeset is required and the primary plane has
> - * a fb, update the members of crtc_state as needed,
> - * and run the necessary updates during vblank evasion.
> - */
> - }
> -
> modeset = needs_modeset(crtc_state);
> recalc = pipe_config->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE;
>
> @@ -15460,47 +15415,22 @@ static void readout_plane_state(struct intel_crtc
> *crtc,
> struct intel_crtc_state *crtc_state)
> {
> struct intel_plane *p;
> - struct drm_plane_state *drm
Re: [Intel-gfx] [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
Hi Dave,
Dave Gordon writes:
> On 13/07/15 14:16, Mika Kuoppala wrote:
>> Arun Siluvery writes:
>>
>>> In Indirect context w/a batch buffer,
>>> +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
>>>
>>> v2: SKL revision id was used for BXT, copy paste error found during
>>> internal review (Bob Beckett).
>>>
>>> Cc: Robert Beckett
>>> Cc: Imre Deak
>>> Signed-off-by: Arun Siluvery
>>> ---
>>> drivers/gpu/drm/i915/intel_lrc.c| 9 +
>>> drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
>>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 58247f0..61ed92b 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct
>>> intel_engine_cs *ring,
>>> struct drm_device *dev = ring->dev;
>>> uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>>>
>>> + /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>>> + if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
>>> + (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
>>> + wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
>>> + wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
>>> + wa_ctx_emit(batch,
>>> _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
>>> + wa_ctx_emit(batch, MI_NOOP);
>>> + }
>>> +
>>> /* WaDisableCtxRestoreArbitration:skl,bxt */
>>> if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
>>> (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index af7c12e..66dde7f 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct
>>> intel_engine_cs *ring)
>>> /*
>>> WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>>> WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
>>> GEN9_RHWO_OPTIMIZATION_DISABLE);
>>> - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
>>> - DISABLE_PIXEL_MASK_CAMMING);
>>> + /*
>>> +* WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be
>>> set,
>>> +* but that register is write only hence it is set
>>> +* in per ctx batch buffer
>>
>> Why the need to move to per ctx bb? Why the write would not stick
>> with this? Is the rationale that we get fails with the gem_workarounds?
>>
>> If so, perhaps we need a write_only marker for workaround list?
>>
>> -Mika
>
> Surely it's the fact that it's a context-saved register that matters,
> rather than it being write-only (generally we treat masked registers as
> write-only anyway, since the masking means that you can set or clear a
> bit without having to read the other bits first).
>
But with workaround list, we push these writes into the ringbuffer
of the context being created. So I still fail to see how they
would end up being out of context.
The per ctx bb would make sense if this register was not part of
contexts and thus would not be saved. Is this the case here?
-Mika
> Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context
> register, so it can be set directly rather than having to be done in a
> batchbuffer.
>
> .Dave.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 10/20] drm/i915: Set csc coefficients in intel_sanitize_crtc.
On Tue, Jul 14, 2015 at 10:50:21AM +0200, Maarten Lankhorst wrote: > Op 13-07-15 om 18:21 schreef Daniel Stone: > > Hi, > > > > On 13 July 2015 at 15:30, Maarten Lankhorst > > wrote: > >> This might not have been set during boot, and when we preserve > >> the initial mode this can result in a black screen. > >> > >> Cc: Daniel Stone > >> Signed-off-by: Maarten Lankhorst > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c > >> b/drivers/gpu/drm/i915/intel_display.c > >> index 819a2b551f1f..80e878929bab 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -15251,6 +15251,9 @@ static void intel_sanitize_crtc(struct intel_crtc > >> *crtc) > >> drm_calc_timestamping_constants(&crtc->base, > >> &crtc->base.hwmode); > >> update_scanline_offset(crtc); > >> drm_crtc_vblank_on(&crtc->base); > >> + > >> + if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen > 7) > >> + intel_set_pipe_csc(&crtc->base); > >> } > > This is a bit of a weird one - and not your fault. > > > > intel_set_pipe_csc is currently only called from haswell_crtc_enable, > > which is only called for gen >= 9 or HAS_DDI. The (IS_HASWELL || gen > > > 7) test covers these devices, plus CHV as well. Does it work on CHV? > I think testing for HAS_DDI is enough, works for skylake too. > > I'd be more tempted to just call this unconditionally, and stick an > > early-exit test in intel_set_pipe_csc instead of at the callsites. But > > intel_set_pipe_csc covers gen >= 6, which can never be triggered > > through haswell_crtc_enable. So, if we added the early-exit to > > set_pipe_csc, we'd also need to remove the previous-gen codepath, or > > add calls to set_pipe_csc which didn't previously exist. > But CSC is only enabled in update_primary_plane for haswell+, so no point in > sanitizing something unused. :-) I think we should instead stick the CSC update into the update_primary_plane hook then? Since before that point we don't care about CSC state at all. Also CSC states need to change atomically with plane updates anyway, so this seems like the right path forward. Can we postpone fixing this until fastboot happens? -Daniel > > > But with the test fixed up to be consistent (gen >= 9 || HAS_DDI): > > Reviewed-by: Daniel Stone > > > > Cheers, > > Daniel > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 15/20] drm/i915: Always reset in intel_crtc_restore_mode
On Mon, Jul 13, 2015 at 04:30:28PM +0200, Maarten Lankhorst wrote:
> And get rid of things that are no longer true. This function is only
> used for forcing a modeset when encoder properties are changed.
>
> Because this is not yet done atomically, assume a full modeset is
> needed and reset the crtc.
>
> Signed-off-by: Maarten Lankhorst
s/reset/force modeset/ in the subject? Reset has a different meaning for
me ...
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 58
> ++--
> 1 file changed, 16 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f6c65706cebf..599af76d34f6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13273,63 +13273,37 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_atomic_state *state;
> - struct intel_encoder *encoder;
> - struct intel_connector *connector;
> - struct drm_connector_state *connector_state;
> - struct intel_crtc_state *crtc_state;
> + struct drm_crtc_state *crtc_state;
> int ret;
>
> state = drm_atomic_state_alloc(dev);
> if (!state) {
> - DRM_DEBUG_KMS("[CRTC:%d] mode restore failed, out of memory",
> + DRM_DEBUG_KMS("[CRTC:%d] crtc restore failed, out of memory",
> crtc->base.id);
> return;
> }
>
> - state->acquire_ctx = dev->mode_config.acquire_ctx;
> -
> - /* The force restore path in the HW readout code relies on the staged
> - * config still keeping the user requested config while the actual
> - * state has been overwritten by the configuration read from HW. We
> - * need to copy the staged config to the atomic state, otherwise the
> - * mode set will just reapply the state the HW is already in. */
> - for_each_intel_encoder(dev, encoder) {
> - if (encoder->base.crtc != crtc)
> - continue;
> + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
>
> - for_each_intel_connector(dev, connector) {
> - if (connector->base.state->best_encoder !=
> - &encoder->base)
> - continue;
> -
> - connector_state = drm_atomic_get_connector_state(state,
> &connector->base);
> - if (IS_ERR(connector_state)) {
> - DRM_DEBUG_KMS("Failed to add [CONNECTOR:%d:%s]
> to state: %ld\n",
> - connector->base.base.id,
> - connector->base.name,
> - PTR_ERR(connector_state));
> - continue;
> - }
> +retry:
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + ret = PTR_ERR_OR_ZERO(crtc_state);
> + if (!ret) {
> + if (!crtc_state->active)
> + goto out;
>
> - connector_state->crtc = crtc;
> - }
> + crtc_state->mode_changed = true;
> + ret = intel_set_mode(state);
> }
>
> - 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));
> - drm_atomic_state_free(state);
> - return;
> + if (ret == -EDEADLK) {
> + drm_atomic_state_clear(state);
> + drm_modeset_backoff(state->acquire_ctx);
> + goto retry;
> }
>
> - 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);
> -
> - ret = intel_set_mode(state);
> if (ret)
> +out:
> drm_atomic_state_free(state);
> }
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 10/20] drm/i915: Set csc coefficients in intel_sanitize_crtc.
Op 14-07-15 om 11:55 schreef Daniel Vetter: > On Tue, Jul 14, 2015 at 10:50:21AM +0200, Maarten Lankhorst wrote: >> Op 13-07-15 om 18:21 schreef Daniel Stone: >>> Hi, >>> >>> On 13 July 2015 at 15:30, Maarten Lankhorst >>> wrote: This might not have been set during boot, and when we preserve the initial mode this can result in a black screen. Cc: Daniel Stone Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/intel_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 819a2b551f1f..80e878929bab 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15251,6 +15251,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode); update_scanline_offset(crtc); drm_crtc_vblank_on(&crtc->base); + + if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen > 7) + intel_set_pipe_csc(&crtc->base); } >>> This is a bit of a weird one - and not your fault. >>> >>> intel_set_pipe_csc is currently only called from haswell_crtc_enable, >>> which is only called for gen >= 9 or HAS_DDI. The (IS_HASWELL || gen > >>> 7) test covers these devices, plus CHV as well. Does it work on CHV? >> I think testing for HAS_DDI is enough, works for skylake too. >>> I'd be more tempted to just call this unconditionally, and stick an >>> early-exit test in intel_set_pipe_csc instead of at the callsites. But >>> intel_set_pipe_csc covers gen >= 6, which can never be triggered >>> through haswell_crtc_enable. So, if we added the early-exit to >>> set_pipe_csc, we'd also need to remove the previous-gen codepath, or >>> add calls to set_pipe_csc which didn't previously exist. >> But CSC is only enabled in update_primary_plane for haswell+, so no point in >> sanitizing something unused. :-) > I think we should instead stick the CSC update into the > update_primary_plane hook then? Since before that point we don't care > about CSC state at all. > > Also CSC states need to change atomically with plane updates anyway, so > this seems like the right path forward. Can we postpone fixing this until > fastboot happens? > Postponing is fine. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Forward all core DRM ioctls to core compat handling
From: Tvrtko Ursulin Previously only core DRM ioctls under the DRM_COMMAND_BASE were being forwarded, but the drm.h header suggests (and reality confirms) ones after (and including) DRM_COMMAND_END should be forwarded as well. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/i915_ioc32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_ioc32.c b/drivers/gpu/drm/i915/i915_ioc32.c index 176de6322e4d..23aa04cded6b 100644 --- a/drivers/gpu/drm/i915/i915_ioc32.c +++ b/drivers/gpu/drm/i915/i915_ioc32.c @@ -204,7 +204,7 @@ long i915_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) drm_ioctl_compat_t *fn = NULL; int ret; - if (nr < DRM_COMMAND_BASE) + if (nr < DRM_COMMAND_BASE || nr >= DRM_COMMAND_END) return drm_compat_ioctl(filp, cmd, arg); if (nr < DRM_COMMAND_BASE + ARRAY_SIZE(i915_compat_ioctls)) -- 2.4.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm: Provide compat ioctl for addfb2.1
From: Tvrtko Ursulin
Frame buffer modifiers extensions provided in;
commit e3eb3250d84ef97b766312345774367b6a310db8
Author: Rob Clark
Date: Thu Feb 5 14:41:52 2015 +
drm: add support for tiled/compressed/etc modifier in addfb2
Missed the structure packing/alignment problem where 64-bit
members were added after the odd number of 32-bit ones. This
makes the compiler produce structures of different sizes under
32- and 64-bit x86 targets and makes the ioctl need explicit
compat handling.
v2: Removed the typedef. (Daniel Vetter)
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Daniel Vetter
Cc: dri-de...@lists.freedesktop.org
Cc: Rob Clark
Cc: Daniel Stone
Cc: Daniel Vetter
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/drm_ioc32.c | 60 +
1 file changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index 8dcfa76b09e6..40d18d6173d9 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -70,6 +70,8 @@
#define DRM_IOCTL_WAIT_VBLANK32DRM_IOWR(0x3a,
drm_wait_vblank32_t)
+#define DRM_IOCTL_MODE_ADDFB232DRM_IOWR(0xb8,
drm_mode_fb_cmd232_t)
+
typedef struct drm_version_32 {
int version_major;/**< Major version */
int version_minor;/**< Minor version */
@@ -1013,6 +1015,63 @@ static int compat_drm_wait_vblank(struct file *file,
unsigned int cmd,
return 0;
}
+struct drm_mode_fb_cmd232 {
+ u32 fb_id;
+ u32 width;
+ u32 height;
+ u32 pixel_format;
+ u32 flags;
+ u32 handles[4];
+ u32 pitches[4];
+ u32 offsets[4];
+ u64 modifier[4];
+} __attribute__((packed));
+
+static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct drm_mode_fb_cmd232 __user *argp = (void __user *)arg;
+ struct drm_mode_fb_cmd232 req32;
+ struct drm_mode_fb_cmd2 __user *req64;
+ int i;
+ int err;
+
+ if (copy_from_user(&req32, argp, sizeof(req32)))
+ return -EFAULT;
+
+ req64 = compat_alloc_user_space(sizeof(*req64));
+
+ if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
+ || __put_user(req32.width, &req64->width)
+ || __put_user(req32.height, &req64->height)
+ || __put_user(req32.pixel_format, &req64->pixel_format)
+ || __put_user(req32.flags, &req64->flags))
+ return -EFAULT;
+
+ for (i = 0; i < 4; i++) {
+ if (__put_user(req32.handles[i], &req64->handles[i]))
+ return -EFAULT;
+ if (__put_user(req32.pitches[i], &req64->pitches[i]))
+ return -EFAULT;
+ if (__put_user(req32.offsets[i], &req64->offsets[i]))
+ return -EFAULT;
+ if (__put_user(req32.modifier[i], &req64->modifier[i]))
+ return -EFAULT;
+ }
+
+ err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
+ if (err)
+ return err;
+
+ if (__get_user(req32.fb_id, &req64->fb_id))
+ return -EFAULT;
+
+ if (copy_to_user(argp, &req32, sizeof(req32)))
+ return -EFAULT;
+
+ return 0;
+}
+
static drm_ioctl_compat_t *drm_compat_ioctls[] = {
[DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
[DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
@@ -1045,6 +1104,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
[DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
#endif
[DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
+ [DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
};
/**
--
2.4.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 00/10] Color Manager Implementation
On Tue, Jul 14, 2015 at 11:35:30AM +0200, Hans Verkuil wrote: > On 07/14/15 11:11, Daniel Vetter wrote: > > On Tue, Jul 14, 2015 at 10:17:09AM +0200, Hans Verkuil wrote: > >> On 07/13/15 16:07, Daniel Vetter wrote: > >>> On Mon, Jul 13, 2015 at 12:11:08PM +0200, Hans Verkuil wrote: > On 07/13/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Jul 13, 2015 at 11:43:31AM +0200, Hans Verkuil wrote: > >> On 07/13/2015 11:18 AM, Daniel Vetter wrote: > >>> On Mon, Jul 13, 2015 at 10:29:32AM +0200, Hans Verkuil wrote: > On 06/15/2015 08:53 AM, Daniel Vetter wrote: > > On Tue, Jun 09, 2015 at 01:50:48PM +0100, Damien Lespiau wrote: > >> On Thu, Jun 04, 2015 at 07:12:31PM +0530, Kausal Malladi wrote: > >>> From: Kausal Malladi > >>> > >>> This patch set adds color manager implementation in drm/i915 > >>> layer. > >>> Color Manager is an extension in i915 driver to support color > >>> correction/enhancement. Various Intel platforms support several > >>> color correction capabilities. Color Manager provides abstraction > >>> of these properties and allows a user space UI agent to > >>> correct/enhance the display. > >> > >> So I did a first rough pass on the API itself. The big question > >> that > >> isn't solved at the moment is: do we want to try to do generic KMS > >> properties for pre-LUT + matrix + post-LUT or not. "Generic" has 3 > >> levels: > >> > >> 1/ Generic for all KMS drivers > >> 2/ Generic for i915 supported platfoms > >> 3/ Specific to each platform > >> > >> At this point, I'm quite tempted to say we should give 1/ a shot. > >> We > >> should be able to have pre-LUT + matrix + post-LUT on CRTC objects > >> and > >> guarantee that, when the drivers expose such properties, user > >> space can > >> at least give 8 bits LUT + 3x3 matrix + 8 bits LUT. > >> > >> It may be possible to use the "try" version of the atomic ioctl to > >> explore the space of possibilities from a generic user space to use > >> bigger LUTs as well. A HAL layer (which is already there in some > >> but not > >> all OSes) would still be able to use those generic properties to > >> load > >> "precision optimized" LUTs with some knowledge of the hardware. > > > > Yeah, imo 1/ should be doable. For the matrix we should be able to > > be > > fully generic with a 16.16 format. For gamma one option would be to > > have > > I know I am late replying, apologies for that. > > I've been working on CSC support for V4L2 as well (still work in > progress) > and I would like to at least end up with the same low-level fixed > point > format as DRM so we can share matrix/vector calculations. > > Based on my experiences I have concerns about the 16.16 format: the > precision > is quite low which can be a problem when such values are used in > matrix > multiplications. > > In addition, while the precision may be sufficient for 8 bit color > component > values, I'm pretty sure it will be insufficient when dealing with 12 > or 16 bit > color components. > > In earlier versions of my CSC code I used a 12.20 format, but in the > latest I > switched to 32.32. This fits nicely in a u64 and it's easy to > extract the > integer and fractional parts. > > If this is going to be a generic and future proof API, then my > suggestion > would be to increase the precision of the underlying data type. > >>> > >>> We discussed this a bit more internally and figured it would be nice > >>> to have the same > >>> fixed point for both CSC matrix and LUT/gamma tables. Current > >>> consensus > >>> seems to be to go with 8.24 for both. Since LUTs are fairly big I > >>> think it > >>> makes sense if we try to be not too wasteful (while still future-proof > >>> ofc). > >> > >> The .24 should have enough precision, but I am worried about the 8: > >> while > >> this works for 8 bit components, you can't use it to represent values > >>> 255, which might be needed (now or in the future) for 10, 12 or 16 bit > >> color components. > >> > >> It's why I ended up with 32.32: it's very generic so usable for other > >> things besides CSC. > >> > >> Note that 8.24 is really 7.24 + one sign bit. So 255 can't be > >> represented > >> in this format. > >> > >> That said, all values I'm working with in my current code are small > >> integer
[Intel-gfx] [PATCH v3.1 04/20] drm/i915: Allow fuzzy matching in pipe_config_compare, v2.
Instead of doing ad-hoc checks we already have a way of checking
if the state is compatible or not. Use this to force a modeset.
Only during modesets, or with PIPE_CONFIG_QUIRK_INHERITED_MODE
we should check if a full modeset is really needed.
Fastboot will allow the adjust parameter to ignore some stuff
too, and it will fix up differences in state that are ignored
by the compare function.
Changes since v1:
- Increase the value of the lowest m/n to prevent truncation.
- Dump pipe config when fastboot's used, without a modeset.
- Add adjust parameter to intel_compare_link_m_n, which is
used to adjust m2_n2 if it's a multiple of m_n.
- Add exact parameter intel_compare_m_n.
Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Stone
---
Fixed the commit message.
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 0eb1cc548656..109d1a5c6c87 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12292,19 +12292,6 @@ encoder_retry:
DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
- /* Check if we need to force a modeset */
- if (pipe_config->has_audio !=
- to_intel_crtc_state(crtc->state)->has_audio) {
- pipe_config->base.mode_changed = true;
- ret = drm_atomic_add_affected_planes(state, crtc);
- }
-
- /*
-* Note we have an issue here with infoframes: current code
-* only updates them on the full mode set path per hw
-* requirements. So here we should be checking for any
-* required changes and forcing a mode set.
-*/
fail:
return ret;
}
@@ -12408,27 +12395,133 @@ static bool intel_fuzzy_clock_check(int clock1, int
clock2)
base.head) \
if (mask & (1 <<(intel_crtc)->pipe))
+
+static bool
+intel_compare_m_n(unsigned int m, unsigned int n,
+ unsigned int m2, unsigned int n2,
+ bool exact)
+{
+ if (m == m2 && n == n2)
+ return true;
+
+ if (exact || !m || !n || !m2 || !n2)
+ return false;
+
+ BUILD_BUG_ON(DATA_LINK_M_N_MASK > INT_MAX);
+
+ if (m > m2) {
+ while (m > m2) {
+ m2 <<= 1;
+ n2 <<= 1;
+ }
+ } else if (m < m2) {
+ while (m < m2) {
+ m <<= 1;
+ n <<= 1;
+ }
+ }
+
+ return m == m2 && n == n2;
+}
+
+static bool
+intel_compare_link_m_n(const struct intel_link_m_n *m_n,
+ struct intel_link_m_n *m2_n2,
+ bool adjust)
+{
+ if (m_n->tu == m2_n2->tu &&
+ intel_compare_m_n(m_n->gmch_m, m_n->gmch_n,
+ m2_n2->gmch_m, m2_n2->gmch_n, !adjust) &&
+ intel_compare_m_n(m_n->link_m, m_n->link_n,
+ m2_n2->link_m, m2_n2->link_n, !adjust)) {
+ if (adjust)
+ *m2_n2 = *m_n;
+
+ return true;
+ }
+
+ return false;
+}
+
static bool
intel_pipe_config_compare(struct drm_device *dev,
struct intel_crtc_state *current_config,
- struct intel_crtc_state *pipe_config)
+ struct intel_crtc_state *pipe_config,
+ bool adjust)
{
+ bool ret = true;
+
+#define INTEL_ERR_OR_DBG_KMS(fmt, ...) \
+ do { \
+ if (!adjust) \
+ DRM_ERROR(fmt, ##__VA_ARGS__); \
+ else \
+ DRM_DEBUG_KMS(fmt, ##__VA_ARGS__); \
+ } while (0)
+
#define PIPE_CONF_CHECK_X(name)\
if (current_config->name != pipe_config->name) { \
- DRM_ERROR("mismatch in " #name " " \
+ INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
"(expected 0x%08x, found 0x%08x)\n", \
current_config->name, \
pipe_config->name); \
- return false; \
+ ret = false; \
}
#define PIPE_CONF_CHECK_I(name)\
if (current_config->name != pipe_config->name) { \
- DRM_ERROR("mismatch in " #name " " \
+ INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
"(expected %i, found %i)\n", \
current_config->name, \
pipe_config->name); \
- return false; \
+ ret = false; \
+ }
+
+#define PIPE_CONF_CHECK_M_N(name) \
+ if (!intel_compare_link_m_n(¤t_config->name, \
+ &pipe_config->name,\
+ adjust)) { \
+ INTEL_ERR_OR_DBG_KMS("mismatch in " #name " " \
+ "(ex
Re: [Intel-gfx] [PATCH 01/18] drm/gem: rip out drm vma accounting for gem mmaps
On Sat, Jul 11, 2015 at 10:11:43PM +0100, Chris Wilson wrote: > On Thu, Jul 09, 2015 at 11:32:33PM +0200, Daniel Vetter wrote: > > Doesn't really add anything which can't be figured out through > > proc files. And more clearly separates the new gem mmap handling > > code from the old drm maps mmap handling code, which is surely a > > good thing. > > > > Cc: Martin Peres > > Signed-off-by: Daniel Vetter > > Reviewed-by: Chris Wilson Applied to topic/drm-misc, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm: Provide compat ioctl for addfb2.1
On Tue, Jul 14, 2015 at 11:13:08AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Frame buffer modifiers extensions provided in;
>
> commit e3eb3250d84ef97b766312345774367b6a310db8
> Author: Rob Clark
> Date: Thu Feb 5 14:41:52 2015 +
>
> drm: add support for tiled/compressed/etc modifier in addfb2
>
> Missed the structure packing/alignment problem where 64-bit
> members were added after the odd number of 32-bit ones. This
> makes the compiler produce structures of different sizes under
> 32- and 64-bit x86 targets and makes the ioctl need explicit
> compat handling.
>
> v2: Removed the typedef. (Daniel Vetter)
>
> Signed-off-by: Tvrtko Ursulin
> Reviewed-by: Daniel Vetter
> Cc: dri-de...@lists.freedesktop.org
> Cc: Rob Clark
> Cc: Daniel Stone
> Cc: Daniel Vetter
> Cc: sta...@vger.kernel.org
Applied to topic/drm-fixes. I pulled in the i915 one (with cc:stable
added) already.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_ioc32.c | 60
> +
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index 8dcfa76b09e6..40d18d6173d9 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -70,6 +70,8 @@
>
> #define DRM_IOCTL_WAIT_VBLANK32 DRM_IOWR(0x3a,
> drm_wait_vblank32_t)
>
> +#define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8,
> drm_mode_fb_cmd232_t)
> +
> typedef struct drm_version_32 {
> int version_major;/**< Major version */
> int version_minor;/**< Minor version */
> @@ -1013,6 +1015,63 @@ static int compat_drm_wait_vblank(struct file *file,
> unsigned int cmd,
> return 0;
> }
>
> +struct drm_mode_fb_cmd232 {
> + u32 fb_id;
> + u32 width;
> + u32 height;
> + u32 pixel_format;
> + u32 flags;
> + u32 handles[4];
> + u32 pitches[4];
> + u32 offsets[4];
> + u64 modifier[4];
> +} __attribute__((packed));
> +
> +static int compat_drm_mode_addfb2(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct drm_mode_fb_cmd232 __user *argp = (void __user *)arg;
> + struct drm_mode_fb_cmd232 req32;
> + struct drm_mode_fb_cmd2 __user *req64;
> + int i;
> + int err;
> +
> + if (copy_from_user(&req32, argp, sizeof(req32)))
> + return -EFAULT;
> +
> + req64 = compat_alloc_user_space(sizeof(*req64));
> +
> + if (!access_ok(VERIFY_WRITE, req64, sizeof(*req64))
> + || __put_user(req32.width, &req64->width)
> + || __put_user(req32.height, &req64->height)
> + || __put_user(req32.pixel_format, &req64->pixel_format)
> + || __put_user(req32.flags, &req64->flags))
> + return -EFAULT;
> +
> + for (i = 0; i < 4; i++) {
> + if (__put_user(req32.handles[i], &req64->handles[i]))
> + return -EFAULT;
> + if (__put_user(req32.pitches[i], &req64->pitches[i]))
> + return -EFAULT;
> + if (__put_user(req32.offsets[i], &req64->offsets[i]))
> + return -EFAULT;
> + if (__put_user(req32.modifier[i], &req64->modifier[i]))
> + return -EFAULT;
> + }
> +
> + err = drm_ioctl(file, DRM_IOCTL_MODE_ADDFB2, (unsigned long)req64);
> + if (err)
> + return err;
> +
> + if (__get_user(req32.fb_id, &req64->fb_id))
> + return -EFAULT;
> +
> + if (copy_to_user(argp, &req32, sizeof(req32)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static drm_ioctl_compat_t *drm_compat_ioctls[] = {
> [DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
> [DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
> @@ -1045,6 +1104,7 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
> [DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
> #endif
> [DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
> + [DRM_IOCTL_NR(DRM_IOCTL_MODE_ADDFB232)] = compat_drm_mode_addfb2,
> };
>
> /**
> --
> 2.4.2
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Revert "drm/i915: Declare the swizzling unknown for L-shaped configurations"
This reverts commit 19ee835cdb0b5a8eb11a68f25a51b8039d564488.
It breaks existing old userspace which doesn't handle UNKNOWN
swizzling correct. Yes UNKNOWN was a thing back in 2009 and probably
still is on some other platforms, but it still pretty clearly broke
the testers machine. If we want this we need to extend the ioctl with
new paramters that only new userspace looks at.
Cc: Harald Arnesen
Cc: Chris Wilson
Reported-by: Harald Arnesen
Cc: sta...@vger.kernel.org
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_gem_tiling.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c
b/drivers/gpu/drm/i915/i915_gem_tiling.c
index d61e74a08f82..633bd1fcab69 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -183,18 +183,8 @@ i915_gem_detect_bit_6_swizzle(struct drm_device *dev)
if (IS_GEN4(dev)) {
uint32_t ddc2 = I915_READ(DCC2);
- if (!(ddc2 & DCC2_MODIFIED_ENHANCED_DISABLE)) {
- /* Since the swizzling may vary within an
-* object, we have no idea what the swizzling
-* is for any page in particular. Thus we
-* cannot migrate tiled pages using the GPU,
-* nor can we tell userspace what the exact
-* swizzling is for any object.
-*/
+ if (!(ddc2 & DCC2_MODIFIED_ENHANCED_DISABLE))
dev_priv->quirks |= QUIRK_PIN_SWIZZLED_PAGES;
- swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN;
- swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN;
- }
}
if (dcc == 0x) {
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 07/20] drm/i915: Rework plane readout.
Op 14-07-15 om 11:53 schreef Daniel Vetter: > On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote: >> All non-primary planes get disabled during hw readout, >> this reduces complexity and means not having to do some plane >> visibility checks during the first commit. >> >> Signed-off-by: Maarten Lankhorst > I still think calling readout_plane_state from the hw state readout code > is the wrong approach. Instead we should consolidate all the plane > readout code exclusively into a new intel_plane_readout_hw_state helper > which is called only from driver load paths. That should also contain the > fb/gem_bo reconstruction loop. Is that a nack? > For the other 2 users of this code (lid notifiery and resume) we should > just force an unconditional plane restore by setting > crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work, > once atomic has settled a bit more. > If you want to force a plane restore, planes_changed won't do what you think it does. planes_changed is a flag that says one or more planes were added to the crtc_state. You want to do drm_atomic_add_affected_planes for that, and atomic resume would do that since it duplicates all plane state. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 08/20] drm/i915: fill in more mode members
Hey, Op 14-07-15 om 11:50 schreef Daniel Vetter: > On Mon, Jul 13, 2015 at 04:30:21PM +0200, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst > Needs a summary (or just pasting relevant parts of our mail threads) about > double-modesets, ways to avoid them and why exactly we ended up opting for > this solution here. Especially please highlight the TYPE_DRIVER trick. Not in this patch I think, that would be for 11/20. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 07/20] drm/i915: Rework plane readout.
On Tue, Jul 14, 2015 at 12:30:38PM +0200, Maarten Lankhorst wrote: > Op 14-07-15 om 11:53 schreef Daniel Vetter: > > On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote: > >> All non-primary planes get disabled during hw readout, > >> this reduces complexity and means not having to do some plane > >> visibility checks during the first commit. > >> > >> Signed-off-by: Maarten Lankhorst > > I still think calling readout_plane_state from the hw state readout code > > is the wrong approach. Instead we should consolidate all the plane > > readout code exclusively into a new intel_plane_readout_hw_state helper > > which is called only from driver load paths. That should also contain the > > fb/gem_bo reconstruction loop. > Is that a nack? Nope, just a "I think we want to clarify this more". I'd welcome a patch on top, but can be done as part of converting dpms (since that will result in lots of cleanups too). > > For the other 2 users of this code (lid notifiery and resume) we should > > just force an unconditional plane restore by setting > > crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work, > > once atomic has settled a bit more. > > > If you want to force a plane restore, planes_changed won't do what you think > it does. > planes_changed is a flag that says one or more planes were added to the > crtc_state. > > You want to do drm_atomic_add_affected_planes for that, and atomic resume > would do that > since it duplicates all plane state. Ah right, so should be possible to just consolidate the plane readout code without requiring any changes in the force_restore case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3.1 06/20] drm/i915: Remove plane_config from struct intel_crtc, v2.
commit 4efb477b56ca9ac6ab76136a2801aaee4d3f46c5
Author: Maarten Lankhorst
Date: Mon Jul 13 15:29:47 2015 +0200
drm/i915: Remove plane_config from struct intel_crtc, v2.
Nothing depends on this outside initial hw readout, so keep this
struct on the stack instead.
Changes since v1:
- Remove unrelated changes.
Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Stone
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6432c180725f..0ca2b1df7e03 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15206,6 +15206,8 @@ void intel_modeset_init(struct drm_device *dev)
drm_modeset_unlock_all(dev);
for_each_intel_crtc(dev, crtc) {
+ struct intel_initial_plane_config plane_config = {};
+
if (!crtc->active)
continue;
@@ -15216,15 +15218,14 @@ void intel_modeset_init(struct drm_device *dev)
* can even allow for smooth boot transitions if the BIOS
* fb is large enough for the active pipe configuration.
*/
- if (dev_priv->display.get_initial_plane_config) {
- dev_priv->display.get_initial_plane_config(crtc,
- &crtc->plane_config);
- /*
-* If the fb is shared between multiple heads, we'll
-* just get the first one.
-*/
- intel_find_initial_plane_obj(crtc, &crtc->plane_config);
- }
+ dev_priv->display.get_initial_plane_config(crtc,
+ &plane_config);
+
+ /*
+* If the fb is shared between multiple heads, we'll
+* just get the first one.
+*/
+ intel_find_initial_plane_obj(crtc, &plane_config);
}
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 09a0a9222a3a..09e3581c8441 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -550,7 +550,6 @@ struct intel_crtc {
uint32_t cursor_size;
uint32_t cursor_base;
- struct intel_initial_plane_config plane_config;
struct intel_crtc_state *config;
bool new_enabled;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 08/20] drm/i915: fill in more mode members
Hi, On 14 July 2015 at 10:50, Daniel Vetter wrote: > On Mon, Jul 13, 2015 at 04:30:21PM +0200, Maarten Lankhorst wrote: >> Signed-off-by: Maarten Lankhorst > > Needs a summary (or just pasting relevant parts of our mail threads) about > double-modesets, ways to avoid them and why exactly we ended up opting for > this solution here. Especially please highlight the TYPE_DRIVER trick. Actually, the original motivation for this patch when it was in my branch was very different: I was seeing failures on commit because hsync == vsync == 0, and also the lack of name made debugging a bit more difficult. TYPE_DRIVER is new though, and separately motivated. :) Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 06/20] drm/i915: Remove plane_config from struct intel_crtc.
Hey,
On 14 July 2015 at 09:27, Maarten Lankhorst
wrote:
> Op 13-07-15 om 18:30 schreef Daniel Stone:
>> On 13 July 2015 at 15:30, Maarten Lankhorst
>> wrote:
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> b/drivers/gpu/drm/i915/intel_display.c
>>> index 037a85f1b127..e4d8acc63823 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -15204,7 +15204,9 @@ void intel_modeset_init(struct drm_device *dev)
>>> drm_modeset_unlock_all(dev);
>>>
>>> for_each_intel_crtc(dev, crtc) {
>>> - if (!crtc->active)
>>> + struct intel_initial_plane_config plane_config;
>>> +
>>> + if (!crtc->base.state->active)
>> Unrelated change from crtc->active to crtc->base.state->active - can
>> we do this in one of the later patches?
> Probably, but I'm trying to get rid of crtc->active every time I touch a
> function.
Sure, but it does make bisection a bit more difficult.
> Eventually this will mean being able to remove it. ;-)
Hey, you know I'm in favour of this!
>>> + plane_config.fb = NULL;
>> memset to 0 instead?
> Well for intel_find_initial_plane_obj it's sufficient, but I can just
> initialize with plane_config = {}; to keep old behavior.
>>> @@ -15713,6 +15716,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
>>> if (ret) {
>>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>>> to_intel_crtc(c)->pipe);
>>> + obj->frontbuffer_bits &=
>>> +
>>> ~to_intel_plane(c->primary)->frontbuffer_bit;
>>> drm_framebuffer_unreference(c->primary->fb);
>>> c->primary->fb = NULL;
>>> c->primary->crtc = c->primary->state->crtc = NULL;
>> Unrelated change?
> Unrelated perhaps, but it fixes a warn when pinning fails.
> Still I guess a WARN won't hurt in that case, I'll drop it.
Yeah, it does make sense to me, but then again it wouldn't be the
first time that a frontbuffer-tracking change I've thought made sense,
actually broke things. ;)
Cheers,
Daniel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect
Sure, Daniel, Thanks. Sonika will send the patches in some time. Regards Shashank -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, July 14, 2015 2:33 PM To: Sharma, Shashank Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not from detect On Tue, Jul 14, 2015 at 08:09:21AM +, Sharma, Shashank wrote: > Please apply this patch series along with the Interrupt handling fix patch > Sonika shared. > With these 4 patches applied, we should not see any problems with HPDs (Until > the HW is broken). Ok, that explains things. If you have depencies please include them all in your patch series and don't put it all over the mailing lists. I won't be able to keep track of everything. I was simply confused about why it should suddenly work without that fix. Can you please resend the entire series with all ingredients required? And enabled on all platforms for maximum testing? Thanks, Daniel > On a similar note, the corresponding chrome team applied the live > status check patch, along with others, and they are not seeing the problems > with HPD, hence they are also interested in this patch series. > > Please let us know if you observe something else, we would love to dig > further. > But as we previously mentioned, this patch series is available and > running across various MCG Gen7 devices, and available with > Gen8 PV production branches also, and there the hotplug stability is pretty > good. > > Regards > Shashank > -Original Message- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > Daniel Vetter > Sent: Tuesday, July 14, 2015 1:29 PM > To: Sharma, Shashank > Cc: Daniel Vetter; Jindal, Sonika; Chris Wilson; > intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and not > from detect > > On Tue, Jul 14, 2015 at 03:48:36AM +, Sharma, Shashank wrote: > > Hi Daniel, Chris > > > > Gen7 and Gen8 platforms have a different live status register (0x61114) and > > we need not to rely on ISR on that. > > As Sonika mentioned, We have been using this register for our commercial > > releases, and found them reliable across a wide range of monitors. > > > > On the other hand, Bsepc clearly mentions, to check the live status before > > even try to read EDID. > > The current DRM nightly code doesn't do that, and we have received few > > errors from Gen7 Chromebooks where you can still read valid EDID on HDMI > > hot-unplug. > > > > So I think this patch and solution is ready, and it should go in. > > I have a gen7 machine here which is currently (and it's somewhat random) > broken wrt hpd and hdmi. And afaics this patch series doesn't have the bugfix > for the hpd handling - or did I miss it? > -Daniel > > > > > Regards > > Shashank > > -Original Message- > > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of > > Daniel Vetter > > Sent: Monday, July 13, 2015 8:27 PM > > To: Jindal, Sonika > > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org; Sharma, Shashank > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Set edid from init and > > not from detect > > > > On Mon, Jul 13, 2015 at 05:29:12PM +0530, Jindal, Sonika wrote: > > > > > > > > > On 7/13/2015 5:10 PM, Chris Wilson wrote: > > > >On Mon, Jul 13, 2015 at 04:19:15PM +0530, Sonika Jindal wrote: > > > >>During init_connector set the edid, then edid will be set/unset > > > >>only during hotplug. For the sake of older platforms where HPD > > > >>is not stable, let edid read happen from detect as well only if it is > > > >>forced to do so. > > > >> > > > >>v2: Removing the 'force' check, instead let detect call read the > > > >>edid for platforms older than gen 7 (Shashank) > > > > > > > >That's enough worse. We now have a random gen check with no > > > >rationale for why you suddenly believe all manufacturers have fixed > > > >their wiring. > > > >There is no reason to believe that gen7 suddenly works is there? > > > >If there is, why don't you mention it? > > > >-Chris > > > > > > > This gen7 check is to be on the safer side not to affect older paltforms. > > > For CHV/VLV, already the live status check is stable enough to > > > determine if we can read edid or not. In VPG production branches > > > we have this patch already available and it was tested with > > > variety of monitors extensively. So we now read the edid only during init > > > and during hotplug. > > > For SKL, the "[PATCH] drm/i915: Handle HPD when it has actually occurred" > > > patch makes HPD stable enough. > > > So, we can rely on the edid read from init_connector instead of > > > reading edid on every detect call. > > > > Again, not going to take this with random gen checks. I want your fix for > > handling hpd on other platforms, then roll this out everywhere. > > -Daniel > > -- > >
Re: [Intel-gfx] [PATCH v3 08/20] drm/i915: fill in more mode members
Op 14-07-15 om 13:21 schreef Daniel Stone: > Hi, > > On 14 July 2015 at 10:50, Daniel Vetter wrote: >> On Mon, Jul 13, 2015 at 04:30:21PM +0200, Maarten Lankhorst wrote: >>> Signed-off-by: Maarten Lankhorst >> Needs a summary (or just pasting relevant parts of our mail threads) about >> double-modesets, ways to avoid them and why exactly we ended up opting for >> this solution here. Especially please highlight the TYPE_DRIVER trick. > Actually, the original motivation for this patch when it was in my > branch was very different: I was seeing failures on commit because > hsync == vsync == 0, and also the lack of name made debugging a bit > more difficult. TYPE_DRIVER is new though, and separately motivated. > Oh, in my branch it was because it made me avoid modesets and to make fastboot work. But fastboot was made to work in a better way. What about: "Fill in driver type, hsync, vrefresh and name. Those members are not read out but can be calculated from the mode." ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On (07/13/15 17:05), Daniel Vetter wrote: > It goes boom somewhere from the cursor ioctl code, which means X is > probably involved. Usual suspects are vt-switching, suspend/resume or > cursor vs. DPMS. You can force a DPMS off from within X with > > $ xset dpms force off > that helped. seems to be working only on -next. [ 42.409154] [ cut here ] [ 42.409234] WARNING: CPU: 0 PID: 359 at drivers/gpu/drm/i915/i915_gem.c:5368 i915_gem_track_fb+0xdc/0x106 [i915]() [ 42.409239] WARN_ON(new->frontbuffer_bits & frontbuffer_bits) [ 42.409243] Modules linked in: [ 42.409248] sha256_ssse3 sha256_generic hmac drbg ctr ccm mousedev arc4 nls_iso8859_1 nls_cp437 coretemp vfat hwmon iwlmvm fat intel_powerclamp crc32c_intel i915 mac80211 psmouse i2c_i801 cfbfillrect cfbimgblt iwlwifi i2c_algo_bit serio_raw cfbcopyarea ie31200_edac lpc_ich atkbd r8169 libps2 mfd_core drm_kms_helper cfg80211 mii drm edac_core thermal mxm_wmi i8042 video serio backlight wmi evdev processor ext4 crc16 mbcache jbd2 sd_mod ehci_pci ehci_hcd ahci libahci libata xhci_pci xhci_hcd scsi_mod usbcore usb_common [ 42.409364] CPU: 0 PID: 359 Comm: Xorg Not tainted 4.2.0-rc2-next-20150713-dbg-00017-g16b87ed-dirty #183 [ 42.409369] 0009 88041ce139d8 813a19ac 81077163 [ 42.409379] 88041ce13a28 88041ce13a18 8103b5d9 88041ce139f8 [ 42.409388] a054b273 0002 88041a938240 88041a938240 [ 42.409397] Call Trace: [ 42.409414] [] dump_stack+0x4c/0x65 [ 42.409425] [] ? up+0x39/0x3e [ 42.409433] [] warn_slowpath_common+0x9b/0xb5 [ 42.409486] [] ? i915_gem_track_fb+0xdc/0x106 [i915] [ 42.409492] [] warn_slowpath_fmt+0x46/0x48 [ 42.409540] [] i915_gem_track_fb+0xdc/0x106 [i915] [ 42.409611] [] intel_prepare_plane_fb+0xb1/0x101 [i915] [ 42.409632] [] drm_atomic_helper_prepare_planes+0x5b/0xb8 [drm_kms_helper] [ 42.409700] [] intel_atomic_commit+0x46/0xc0 [i915] [ 42.409750] [] drm_atomic_commit+0x4d/0x52 [drm] [ 42.409769] [] drm_atomic_helper_update_plane+0xca/0x119 [drm_kms_helper] [ 42.409811] [] __setplane_internal+0x24e/0x2ae [drm] [ 42.409846] [] drm_mode_cursor_universal+0x149/0x197 [drm] [ 42.409880] [] ? drm_mode_setcrtc+0x428/0x428 [drm] [ 42.409910] [] drm_mode_cursor_common+0xb5/0x156 [drm] [ 42.409939] [] drm_mode_cursor_ioctl+0x37/0x39 [drm] [ 42.409967] [] drm_ioctl+0x287/0x415 [drm] [ 42.409975] [] ? __lock_is_held+0x3c/0x57 [ 42.409983] [] ? __fget+0x170/0x1a1 [ 42.409991] [] do_vfs_ioctl+0x455/0x4dd [ 42.409996] [] ? __fget_light+0x65/0x75 [ 42.410003] [] SyS_ioctl+0x44/0x63 [ 42.410010] [] entry_SYSCALL_64_fastpath+0x12/0x6f [ 42.410016] ---[ end trace 90f0a9050a7baa50 ]--- -ss > For suspend resume it occasionally matters whether you initiate it through > the gui or by closing the lid or through timeout (if you're not connected > to a wallplug). I hope this helps with figuring out a repro recipe. > > It could also be a race somewhere, in which case you won't be able to > consistently reproduce this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 09/20] drm/i915: Fill in more crtc state, v3.
Op 14-07-15 om 11:49 schreef Daniel Vetter: > On Mon, Jul 13, 2015 at 04:30:22PM +0200, Maarten Lankhorst wrote: >> There is a small memory leak in intel_modeset_readout_hw_state, >> plug it. > This should be a separate patch I think since it seems unrelated to the > other changes. And I think less mystery in the commit message helps, e.g. > "We leak crtc state references in the hw state readout, fix this." instead > of talking about an onimous small leak ;-) > Argh, you caught me on my laziness to split it out to separate patches. Now I have no choice but to properly split and motivate it! ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Fix reference leak in intel_modeset_readout_hw_state.
Unreference the old mode_blob by calling the crtc_destroy_state
helper before zeroing the crtc_state.
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 14ed5e69f275..4934261c8277 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15450,6 +15450,7 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
int i;
for_each_intel_crtc(dev, crtc) {
+ __drm_atomic_helper_crtc_destroy_state(&crtc->base,
crtc->base.state);
memset(crtc->config, 0, sizeof(*crtc->config));
crtc->config->base.crtc = &crtc->base;
--
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: Zero the mode in intel_sanitize_crtc when force disabling.
There is a WARN_ON in drm_atomic_crtc_check for this when exposing the atomic property. If the mode_blob still exists, but enable = false then all updates are rejected with -EINVAL. Signed-off-by: Maarten Lankhorst --- 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 4934261c8277..a8c8df779eb3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15300,7 +15300,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) crtc->base.state->enable ? "enabled" : "disabled", crtc->active ? "enabled" : "disabled"); - crtc->base.state->enable = crtc->active; + WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) < 0); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; -- 2.1.0 ___ 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: Fix reference leak in intel_modeset_readout_hw_state.
On 14 July 2015 at 12:42, Maarten Lankhorst wrote: > Unreference the old mode_blob by calling the crtc_destroy_state > helper before zeroing the crtc_state. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Daniel Stone ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Calculate vblank timestamping constants before enabling vblank.
This is required to properly initialize vblanks on the active crtc.
Without it drm_calc_vbltimestamp_from_scanoutpos can fail with
crtc 0: Noop due to uninitialized mode.
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 a8c8df779eb3..f434a03e12fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15248,6 +15248,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
/* restore vblank interrupts to correct state */
drm_crtc_vblank_reset(&crtc->base);
if (crtc->active) {
+ drm_calc_timestamping_constants(&crtc->base,
&crtc->base.hwmode);
update_scanline_offset(crtc);
drm_crtc_vblank_on(&crtc->base);
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Zero the mode in intel_sanitize_crtc when force disabling.
On 14 July 2015 at 12:45, Maarten Lankhorst wrote: > There is a WARN_ON in drm_atomic_crtc_check for this when exposing the atomic > property. > If the mode_blob still exists, but enable = false then all updates are > rejected with -EINVAL. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Daniel Stone ___ 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: Calculate vblank timestamping constants before enabling vblank.
On 14 July 2015 at 12:46, Maarten Lankhorst wrote: > This is required to properly initialize vblanks on the active crtc. > Without it drm_calc_vbltimestamp_from_scanoutpos can fail with > crtc 0: Noop due to uninitialized mode. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Daniel Stone ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3.1 06/20] drm/i915: Remove plane_config from struct intel_crtc, v2.
On Tue, Jul 14, 2015 at 12:33:29PM +0200, Maarten Lankhorst wrote:
> commit 4efb477b56ca9ac6ab76136a2801aaee4d3f46c5
> Author: Maarten Lankhorst
> Date: Mon Jul 13 15:29:47 2015 +0200
>
> drm/i915: Remove plane_config from struct intel_crtc, v2.
>
> Nothing depends on this outside initial hw readout, so keep this
> struct on the stack instead.
>
> Changes since v1:
> - Remove unrelated changes.
>
> Signed-off-by: Maarten Lankhorst
> Reviewed-by: Daniel Stone
Merged up to this one (with the strange indent removed).
Thanks, Daniel
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 6432c180725f..0ca2b1df7e03 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15206,6 +15206,8 @@ void intel_modeset_init(struct drm_device *dev)
> drm_modeset_unlock_all(dev);
>
> for_each_intel_crtc(dev, crtc) {
> + struct intel_initial_plane_config plane_config = {};
> +
> if (!crtc->active)
> continue;
>
> @@ -15216,15 +15218,14 @@ void intel_modeset_init(struct drm_device *dev)
>* can even allow for smooth boot transitions if the BIOS
>* fb is large enough for the active pipe configuration.
>*/
> - if (dev_priv->display.get_initial_plane_config) {
> - dev_priv->display.get_initial_plane_config(crtc,
> -&crtc->plane_config);
> - /*
> - * If the fb is shared between multiple heads, we'll
> - * just get the first one.
> - */
> - intel_find_initial_plane_obj(crtc, &crtc->plane_config);
> - }
> + dev_priv->display.get_initial_plane_config(crtc,
> +&plane_config);
> +
> + /*
> + * If the fb is shared between multiple heads, we'll
> + * just get the first one.
> + */
> + intel_find_initial_plane_obj(crtc, &plane_config);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 09a0a9222a3a..09e3581c8441 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -550,7 +550,6 @@ struct intel_crtc {
> uint32_t cursor_size;
> uint32_t cursor_base;
>
> - struct intel_initial_plane_config plane_config;
> struct intel_crtc_state *config;
> bool new_enabled;
>
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] HDMI optimization series
This series adds some optimization related to reading of edid only when required and when live status says so. The comments in the patches explain more. v2: Some refactoring is with this series. Also, right now this is done for platforms gen7 and above because we couldn't test with older platforms. For newer platforms it works reliably. For HPD and live status to work on SKL, following patch is required: "drm/i915: Handle HPD when it has actually occurred" Shashank Sharma (2): drm/i915: add attached connector to hdmi container drm/i915: Add HDMI probe function Sonika Jindal (1): drm/i915: Check live status before reading edid drivers/gpu/drm/i915/intel_dp.c |4 +- drivers/gpu/drm/i915/intel_drv.h |3 ++ drivers/gpu/drm/i915/intel_hdmi.c | 93 + 3 files changed, 88 insertions(+), 12 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Add HDMI probe function
From: Shashank Sharma
This patch adds a separate probe function for HDMI
EDID read over DDC channel. This function has been
registered as a .hot_plug handler for HDMI encoder.
The current implementation of hdmi_detect()
function re-sets the cached HDMI edid (in connector->detect_edid) in
every detect call.This function gets called many times, sometimes
directly from userspace probes, forcing drivers to read EDID every
detect function call.This causes several problems like:
1. Race conditions in multiple hot_plug / unplug cases, between
interrupts bottom halves and userspace detections.
2. Many Un-necessary EDID reads for single hotplug/unplug
3. HDMI complaince failures which expects only one EDID read per hotplug
This function will be serving the purpose of really reading the EDID
by really probing the DDC channel, and updating the cached EDID.
The plan is to:
1. i915 IRQ handler bottom half function already calls
intel_encoder->hotplug() function. Adding This probe function which
will read the EDID only in case of a hotplug / unplug.
2. During init_connector this probe will be called to read the edid
3. Reuse the cached EDID in hdmi_detect() function.
The "< gen7" check is there because this was tested only for >=gen7
platforms. For older platforms the hotplug/reading edid path remains same.
Signed-off-by: Shashank Sharma
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_hdmi.c | 49 -
1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index af4d1e6..c4b82ce 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1340,23 +1340,56 @@ intel_hdmi_set_edid(struct drm_connector *connector)
return connected;
}
+void intel_hdmi_probe(struct intel_encoder *intel_encoder)
+{
+ struct intel_hdmi *intel_hdmi =
+ enc_to_intel_hdmi(&intel_encoder->base);
+ struct intel_connector *intel_connector =
+ intel_hdmi->attached_connector;
+ /*
+* We are here, means there is a hotplug or a force
+* detection. Clear the cached EDID and probe the
+* DDC bus to check the current status of HDMI.
+*/
+ intel_hdmi_unset_edid(&intel_connector->base);
+ if (intel_hdmi_set_edid(&intel_connector->base))
+ DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
+ else
+ DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
+}
+
static enum drm_connector_status
intel_hdmi_detect(struct drm_connector *connector, bool force)
{
enum drm_connector_status status;
+ struct intel_connector *intel_connector =
+ to_intel_connector(connector);
+ struct drm_device *dev = connector->dev;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, connector->name);
+ /*
+* There are many userspace calls which probe EDID from
+* detect path. In case of multiple hotplug/unplug, these
+* can cause race conditions while probing EDID. Also its
+* waste of CPU cycles to read the EDID again and again
+* unless there is a real hotplug.
+* So, reading edid in detect only for older platforms (< gen7)
+* Letting the newer platforms rely on hotplugs and init to read edid.
+* Check connector status based on availability of cached EDID.
+*/
+ if (INTEL_INFO(dev)->gen < 7)
+ intel_hdmi_probe(intel_connector->encoder);
- intel_hdmi_unset_edid(connector);
-
- if (intel_hdmi_set_edid(connector)) {
+ if (intel_connector->detect_edid) {
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
- hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
status = connector_status_connected;
- } else
+ hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+ DRM_DEBUG_DRIVER("hdmi status = connected\n");
+ } else {
status = connector_status_disconnected;
+ DRM_DEBUG_DRIVER("hdmi status = disconnected\n");
+ }
return status;
}
@@ -1997,11 +2030,15 @@ void intel_hdmi_init_connector(struct
intel_digital_port *intel_dig_port,
intel_connector->unregister = intel_connector_unregister;
intel_hdmi_add_properties(intel_hdmi, connector);
+ intel_encoder->hot_plug = intel_hdmi_probe;
intel_connector_attach_encoder(intel_connector, intel_encoder);
drm_connector_register(connector);
intel_hdmi->attached_connector = intel_connector;
+ /* Set edid during init */
+ intel_hdmi_probe(intel_encoder);
+
/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
* 0xd. Failure to do so will result in spurious interrupts being
* generate
[Intel-gfx] [PATCH 3/3] drm/i915: Check live status before reading edid
Adding this for SKL onwards.
v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
to check digital port status. Adding a separate function to get bxt live
status (Daniel)
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_dp.c |4 ++--
drivers/gpu/drm/i915/intel_drv.h |2 ++
drivers/gpu/drm/i915/intel_hdmi.c | 43 +
3 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4ebfc3a..7b54b9d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
return intel_dp_detect_dpcd(intel_dp);
}
-static int g4x_digital_port_connected(struct drm_device *dev,
- struct intel_digital_port
*intel_dig_port)
+int g4x_digital_port_connected(struct drm_device *dev,
+ struct intel_digital_port *intel_dig_port)
{
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t bit;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a47fbc3..30c8dd6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
void intel_edp_drrs_invalidate(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+int g4x_digital_port_connected(struct drm_device *dev,
+ struct intel_digital_port *intel_dig_port);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int
conn_id);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index c4b82ce..402be54 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
to_intel_connector(connector)->detect_edid = NULL;
}
+static bool bxt_port_connected(struct drm_i915_private *dev_priv,
+ struct intel_digital_port *port)
+{
+ u32 temp = I915_READ(GEN8_DE_PORT_ISR);
+
+ /* TODO: Add bxt A0/A1 wa related to hpd pin swap */
+ switch (port->port) {
+ case PORT_B:
+ return temp & BXT_DE_PORT_HP_DDIB;
+
+ case PORT_C:
+ return temp & BXT_DE_PORT_HP_DDIC;
+
+ default:
+ return false;
+
+ }
+}
+
+static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
+{
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+
+ if (IS_VALLEYVIEW(dev))
+ return g4x_digital_port_connected(dev, intel_dig_port);
+ else if (IS_SKYLAKE(dev))
+ return ibx_digital_port_connected(dev_priv, intel_dig_port);
+ else if (IS_BROXTON(dev))
+ return bxt_port_connected(dev_priv, intel_dig_port);
+
+ return true;
+}
+
static bool
intel_hdmi_set_edid(struct drm_connector *connector)
{
@@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
struct intel_encoder *intel_encoder =
&hdmi_to_dig_port(intel_hdmi)->base;
enum intel_display_power_domain power_domain;
- struct edid *edid;
+ struct edid *edid = NULL;
bool connected = false;
power_domain = intel_display_port_power_domain(intel_encoder);
intel_display_power_get(dev_priv, power_domain);
- edid = drm_get_edid(connector,
- intel_gmbus_get_adapter(dev_priv,
- intel_hdmi->ddc_bus));
+ if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
+ edid = drm_get_edid(connector,
+ intel_gmbus_get_adapter(dev_priv,
+ intel_hdmi->ddc_bus));
intel_display_power_put(dev_priv, power_domain);
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: add attached connector to hdmi container
From: Shashank Sharma
This patch adds the intel_connector initialized to intel_hdmi
display, during the init phase, just like the other encoders do.
This attachment is very useful when we need to extract the connector
pointer during the hotplug handler function
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_drv.h |1 +
drivers/gpu/drm/i915/intel_hdmi.c |1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e90c743..a47fbc3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -659,6 +659,7 @@ struct intel_hdmi {
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
enum hdmi_picture_aspect aspect_ratio;
+ struct intel_connector *attached_connector;
void (*write_infoframe)(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 00c4b40..af4d1e6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2000,6 +2000,7 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
intel_connector_attach_encoder(intel_connector, intel_encoder);
drm_connector_register(connector);
+ intel_hdmi->attached_connector = intel_connector;
/* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be written
* 0xd. Failure to do so will result in spurious interrupts being
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3.1 08/20] drm/i915: fill in more mode members
Fill in driver type, hsync, vrefresh and name. Those members are not read out but can be calculated from the mode. Signed-off-by: Maarten Lankhorst --- diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 434b43054d60..fa1cdefa5662 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7756,9 +7756,14 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, mode->vsync_end = pipe_config->base.adjusted_mode.crtc_vsync_end; mode->flags = pipe_config->base.adjusted_mode.flags; + mode->type = DRM_MODE_TYPE_DRIVER; mode->clock = pipe_config->base.adjusted_mode.crtc_clock; mode->flags |= pipe_config->base.adjusted_mode.flags; + + mode->hsync = drm_mode_hsync(mode); + mode->vrefresh = drm_mode_vrefresh(mode); + drm_mode_set_name(mode); } static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3.1 08/20] drm/i915: fill in more mode members
On 14 July 2015 at 13:12, Maarten Lankhorst wrote: > Fill in driver type, hsync, vrefresh and name. > Those members are not read out but can be calculated from the mode. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Daniel Stone Should be safe to merge independently. Cheers, Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl+: Add YUV pixel format in Capability list
GEN >= 9 supports YUV format for all planes, but it's not exported in
Capability list of primary plane. Add YUV formats in skl_primary_formats
list.
Don't rely on fb->bits_per_pixel as intel_framebuffer_init is not
filling bits_per_pixel field of fb-struct for YUV pixel format.
This leads to divide by zero error during watermark calculation.
Signed-off-by: Kumar, Mahesh
Cc: Konduru, Chandra
---
drivers/gpu/drm/i915/intel_display.c | 4
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bb58cb6..f4b27af 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -72,6 +72,10 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_ABGR,
DRM_FORMAT_XRGB2101010,
DRM_FORMAT_XBGR2101010,
+ DRM_FORMAT_YUYV,
+ DRM_FORMAT_YVYU,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_VYUY,
};
/* Cursor formats */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f2be1ce..1d13b7e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3156,8 +3156,8 @@ static void skl_compute_wm_pipe_parameters(struct
drm_crtc *crtc,
/* For planar: Bpp is for uv plane, y_Bpp is for y plane */
if (fb) {
p->plane[0].enabled = true;
- p->plane[0].bytes_per_pixel = fb->pixel_format ==
DRM_FORMAT_NV12 ?
- drm_format_plane_cpp(fb->pixel_format, 1) :
fb->bits_per_pixel / 8;
+ p->plane[0].bytes_per_pixel =
+ drm_format_plane_cpp(fb->pixel_format, 1);
p->plane[0].y_bytes_per_pixel = fb->pixel_format ==
DRM_FORMAT_NV12 ?
drm_format_plane_cpp(fb->pixel_format, 0) : 0;
p->plane[0].tiling = fb->modifier[0];
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 08/20] drm/i915: fill in more mode members
On Tue, Jul 14, 2015 at 01:40:45PM +0200, Maarten Lankhorst wrote: > Op 14-07-15 om 13:21 schreef Daniel Stone: > > Hi, > > > > On 14 July 2015 at 10:50, Daniel Vetter wrote: > >> On Mon, Jul 13, 2015 at 04:30:21PM +0200, Maarten Lankhorst wrote: > >>> Signed-off-by: Maarten Lankhorst > >> Needs a summary (or just pasting relevant parts of our mail threads) about > >> double-modesets, ways to avoid them and why exactly we ended up opting for > >> this solution here. Especially please highlight the TYPE_DRIVER trick. > > Actually, the original motivation for this patch when it was in my > > branch was very different: I was seeing failures on commit because > > hsync == vsync == 0, and also the lack of name made debugging a bit > > more difficult. TYPE_DRIVER is new though, and separately motivated. > > > Oh, in my branch it was because it made me avoid modesets and to make > fastboot work. But fastboot was made to work in a better way. > > What about: > "Fill in driver type, hsync, vrefresh and name. > Those members are not read out but can be calculated from the mode." Maybe add "Note that we mark the mode as TYPE_DRIVER to force a mismatch with whatever mode userspace provides. This ensures we force a modeset on the first real modeset, while avoiding spurious modesets on no-op property changes before that (where the mode is just duplicated exactly)." -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On Tue, Jul 14, 2015 at 08:39:50PM +0900, Sergey Senozhatsky wrote: > On (07/13/15 17:05), Daniel Vetter wrote: > > It goes boom somewhere from the cursor ioctl code, which means X is > > probably involved. Usual suspects are vt-switching, suspend/resume or > > cursor vs. DPMS. You can force a DPMS off from within X with > > > > $ xset dpms force off > > > > that helped. seems to be working only on -next. You mean you only get a backtrace on -next, right? Otherwise I'd be confused ;-) Next up. Please boot with drm.debug=0xe, repro the issue and attach complete dmesg (from boot-up up to the WARNING). That should help us reconstruct how things went wrong here. Thanks, Daniel > > [ 42.409154] [ cut here ] > [ 42.409234] WARNING: CPU: 0 PID: 359 at > drivers/gpu/drm/i915/i915_gem.c:5368 i915_gem_track_fb+0xdc/0x106 [i915]() > [ 42.409239] WARN_ON(new->frontbuffer_bits & frontbuffer_bits) > [ 42.409243] Modules linked in: > [ 42.409248] sha256_ssse3 sha256_generic hmac drbg ctr ccm mousedev arc4 > nls_iso8859_1 nls_cp437 coretemp vfat hwmon iwlmvm fat intel_powerclamp > crc32c_intel i915 mac80211 psmouse i2c_i801 cfbfillrect cfbimgblt iwlwifi > i2c_algo_bit serio_raw cfbcopyarea ie31200_edac lpc_ich atkbd r8169 libps2 > mfd_core drm_kms_helper cfg80211 mii drm edac_core thermal mxm_wmi i8042 > video serio backlight wmi evdev processor ext4 crc16 mbcache jbd2 sd_mod > ehci_pci ehci_hcd ahci libahci libata xhci_pci xhci_hcd scsi_mod usbcore > usb_common > [ 42.409364] CPU: 0 PID: 359 Comm: Xorg Not tainted > 4.2.0-rc2-next-20150713-dbg-00017-g16b87ed-dirty #183 > [ 42.409369] 0009 88041ce139d8 813a19ac > 81077163 > [ 42.409379] 88041ce13a28 88041ce13a18 8103b5d9 > 88041ce139f8 > [ 42.409388] a054b273 0002 88041a938240 > 88041a938240 > [ 42.409397] Call Trace: > [ 42.409414] [] dump_stack+0x4c/0x65 > [ 42.409425] [] ? up+0x39/0x3e > [ 42.409433] [] warn_slowpath_common+0x9b/0xb5 > [ 42.409486] [] ? i915_gem_track_fb+0xdc/0x106 [i915] > [ 42.409492] [] warn_slowpath_fmt+0x46/0x48 > [ 42.409540] [] i915_gem_track_fb+0xdc/0x106 [i915] > [ 42.409611] [] intel_prepare_plane_fb+0xb1/0x101 [i915] > [ 42.409632] [] > drm_atomic_helper_prepare_planes+0x5b/0xb8 [drm_kms_helper] > [ 42.409700] [] intel_atomic_commit+0x46/0xc0 [i915] > [ 42.409750] [] drm_atomic_commit+0x4d/0x52 [drm] > [ 42.409769] [] > drm_atomic_helper_update_plane+0xca/0x119 [drm_kms_helper] > [ 42.409811] [] __setplane_internal+0x24e/0x2ae [drm] > [ 42.409846] [] drm_mode_cursor_universal+0x149/0x197 > [drm] > [ 42.409880] [] ? drm_mode_setcrtc+0x428/0x428 [drm] > [ 42.409910] [] drm_mode_cursor_common+0xb5/0x156 [drm] > [ 42.409939] [] drm_mode_cursor_ioctl+0x37/0x39 [drm] > [ 42.409967] [] drm_ioctl+0x287/0x415 [drm] > [ 42.409975] [] ? __lock_is_held+0x3c/0x57 > [ 42.409983] [] ? __fget+0x170/0x1a1 > [ 42.409991] [] do_vfs_ioctl+0x455/0x4dd > [ 42.409996] [] ? __fget_light+0x65/0x75 > [ 42.410003] [] SyS_ioctl+0x44/0x63 > [ 42.410010] [] entry_SYSCALL_64_fastpath+0x12/0x6f > [ 42.410016] ---[ end trace 90f0a9050a7baa50 ]--- > > -ss > > > For suspend resume it occasionally matters whether you initiate it through > > the gui or by closing the lid or through timeout (if you're not connected > > to a wallplug). I hope this helps with figuring out a repro recipe. > > > > It could also be a race somewhere, in which case you won't be able to > > consistently reproduce this. > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl+: Add YUV pixel format in Capability list
On Tue, Jul 14, 2015 at 06:08:06PM +0530, Kumar, Mahesh wrote:
> GEN >= 9 supports YUV format for all planes, but it's not exported in
> Capability list of primary plane. Add YUV formats in skl_primary_formats
> list.
> Don't rely on fb->bits_per_pixel as intel_framebuffer_init is not
> filling bits_per_pixel field of fb-struct for YUV pixel format.
> This leads to divide by zero error during watermark calculation.
>
> Signed-off-by: Kumar, Mahesh
> Cc: Konduru, Chandra
> ---
> drivers/gpu/drm/i915/intel_display.c | 4
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index bb58cb6..f4b27af 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -72,6 +72,10 @@ static const uint32_t skl_primary_formats[] = {
> DRM_FORMAT_ABGR,
> DRM_FORMAT_XRGB2101010,
> DRM_FORMAT_XBGR2101010,
> + DRM_FORMAT_YUYV,
> + DRM_FORMAT_YVYU,
> + DRM_FORMAT_UYVY,
> + DRM_FORMAT_VYUY,
Needs testcase (or reworking an existing testcase to also test the primary
plane). Also we should really aim to unify all the skl plane functions ...
-Daniel
> };
>
> /* Cursor formats */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f2be1ce..1d13b7e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3156,8 +3156,8 @@ static void skl_compute_wm_pipe_parameters(struct
> drm_crtc *crtc,
> /* For planar: Bpp is for uv plane, y_Bpp is for y plane */
> if (fb) {
> p->plane[0].enabled = true;
> - p->plane[0].bytes_per_pixel = fb->pixel_format ==
> DRM_FORMAT_NV12 ?
> - drm_format_plane_cpp(fb->pixel_format, 1) :
> fb->bits_per_pixel / 8;
> + p->plane[0].bytes_per_pixel =
> + drm_format_plane_cpp(fb->pixel_format, 1);
> p->plane[0].y_bytes_per_pixel = fb->pixel_format ==
> DRM_FORMAT_NV12 ?
> drm_format_plane_cpp(fb->pixel_format, 0) : 0;
> p->plane[0].tiling = fb->modifier[0];
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
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: Remove unused compat32 code
On Tue, Jul 14, 2015 at 10:15:21AM +0100, Chris Wilson wrote: > On Tue, Jul 14, 2015 at 10:59:30AM +0200, Daniel Vetter wrote: > > Totatlly forgotten that we have these when nuking all the UMS code. > > > > Signed-off-by: Daniel Vetter > > All deleted UMS stubs, > Reviewed-by: Chris Wilson Applied to dinq, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/gen9: Implement WaDisableKillLogic for gen 9
Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 709b3c7..71ef18c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2880,7 +2880,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) spin_lock_irqsave(&ring->execlist_lock, flags); if (ctx_obj && (ctx != ring->default_context)) - intel_lr_context_unpin(ring, ctx); + intel_lr_context_unpin(request); intel_runtime_pm_put(dev_priv); spin_unlock_irqrestore(&ring->execlist_lock, flags); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 059de0f..afa8972 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -160,6 +160,7 @@ #define GAM_ECOCHK 0x4090 #define BDW_DISABLE_HDC_INVALIDATION (1<<25) #define ECOCHK_SNB_BIT (1<<10) +#define ECOCHK_DIS_TLB (1<<8) #define HSW_ECOCHK_ARB_PRIO_SOL (1<<6) #define ECOCHK_PPGTT_CACHE64B(0x3<<3) #define ECOCHK_PPGTT_CACHE4B (0x0<<3) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f2be1ce..6986342 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -59,6 +59,10 @@ static void gen9_init_clock_gating(struct drm_device *dev) /* WaEnableLbsSlaRetryTimerDecrement:skl */ I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); + + /* WaDisableKillLogic:bxt,skl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + ECOCHK_DIS_TLB); } static void skl_init_clock_gating(struct drm_device *dev) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On (07/14/15 14:44), Daniel Vetter wrote: > > that helped. seems to be working only on -next. > > You mean you only get a backtrace on -next, right? yeah, sure :-) > Otherwise I'd be confused ;-) > > Next up. Please boot with drm.debug=0xe, repro the issue and attach > complete dmesg (from boot-up up to the WARNING). That should help us > reconstruct how things went wrong here. can't reproduce it thus far. sometimes `xset dpms force off' just turns off the panel for a second, sometimes -- until I generate a `wakeup' event (key press, etc.) part of dmesg (no WARNING yet) [ 253.699215] [drm:check_encoder_state] [ENCODER:32:DAC-32] [ 253.699217] [drm:check_encoder_state] [ENCODER:33:TMDS-33] [ 253.699219] [drm:check_encoder_state] [ENCODER:41:TMDS-41] [ 253.699221] [drm:check_crtc_state] [CRTC:21] [ 253.699225] [drm:check_crtc_state] [CRTC:25] [ 253.699226] [drm:check_crtc_state] [CRTC:29] [ 253.699228] [drm:check_shared_dpll_state] WRPLL 1 [ 253.699230] [drm:check_shared_dpll_state] WRPLL 2 [ 253.699270] [drm:intel_backlight_device_update_status] updating intel_backlight, brightness=0/976 [ 256.612288] [drm:drm_mode_setcrtc] [CRTC:21] [ 256.612299] [drm:drm_mode_setcrtc] [CONNECTOR:34:eDP-1] [ 256.612302] [drm:intel_crtc_set_config] [CRTC:21] [FB:45] #connectors=1 (x y) (0 0) [ 256.612308] [drm:intel_modeset_stage_output_state] [CONNECTOR:34:eDP-1] to [CRTC:21] [ 256.612317] [drm:connected_sink_compute_bpp] [CONNECTOR:34:eDP-1] checking for sink bpp constrains [ 256.612318] [drm:connected_sink_compute_bpp] clamping display bpp (was 36) to EDID reported max of 18 [ 256.612322] [drm:intel_dp_compute_config] DP link computation with max lane count 2 max bw 27 pixel clock 143000KHz [ 256.612324] [drm:intel_dp_compute_config] DP link bw 0a lane count 2 clock 27 bpp 18 [ 256.612326] [drm:intel_dp_compute_config] DP link bw required 257400 available 432000 [ 256.612328] [drm:intel_modeset_pipe_config] plane bpp: 36, pipe bpp: 18, dithering: 1 [ 256.612331] [drm:intel_dump_pipe_config] [CRTC:21][modeset] config 8800c6683400 for pipe A [ 256.612332] [drm:intel_dump_pipe_config] cpu_transcoder: D [ 256.612333] [drm:intel_dump_pipe_config] pipe bpp: 18, dithering: 1 [ 256.612335] [drm:intel_dump_pipe_config] fdi/pch: 0, lanes: 0, gmch_m: 0, gmch_n: 0, link_m: 0, link_n: 0, tu: 0 [ 256.612337] [drm:intel_dump_pipe_config] dp: 1, gmch_m: 4998212, gmch_n: 8388608, link_m: 277678, link_n: 524288, tu: 64 [ 256.612339] [drm:intel_dump_pipe_config] dp: 1, gmch_m2: 0, gmch_n2: 0, link_m2: 0, link_n2: 0, tu2: 0 [ 256.612340] [drm:intel_dump_pipe_config] audio: 0, infoframes: 0 [ 256.612342] [drm:intel_dump_pipe_config] requested mode: [ 256.612344] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 60 143000 1920 1968 2000 2080 1080 1082 1087 1144 0x0 0xa [ 256.612346] [drm:intel_dump_pipe_config] adjusted mode: [ 256.612348] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 60 143000 1920 1968 2000 2080 1080 1082 1087 1144 0x48 0xa [ 256.612350] [drm:intel_dump_crtc_timings] crtc timings: 143000 1920 1968 2000 2080 1080 1082 1087 1144, type: 0x48 flags: 0xa [ 256.612351] [drm:intel_dump_pipe_config] port clock: 27 [ 256.612353] [drm:intel_dump_pipe_config] pipe src size: 1920x1080 [ 256.612354] [drm:intel_dump_pipe_config] num_scalers: 0, scaler_users: 0x0, scaler_id: 0 [ 256.612356] [drm:intel_dump_pipe_config] gmch pfit: control: 0x, ratios: 0x, lvds border: 0x [ 256.612358] [drm:intel_dump_pipe_config] pch pfit: pos: 0x, size: 0x, disabled [ 256.612359] [drm:intel_dump_pipe_config] ips: 0 [ 256.612360] [drm:intel_dump_pipe_config] double wide: 0 [ 256.612362] [drm:intel_dump_pipe_config] ddi_pll_sel: 536870912; dpll_hw_state: wrpll: 0x0 [ 256.612363] [drm:intel_dump_pipe_config] planes on this crtc [ 256.612365] [drm:intel_dump_pipe_config] STANDARD PLANE:18 plane: 0.0 idx: 0 enabled [ 256.612367] [drm:intel_dump_pipe_config] FB:45, fb = 1920x1080 format = 0x34325258 [ 256.612369] [drm:intel_dump_pipe_config] scaler:0 src (0, 0) 0x0 dst (0, 0) 0x0 [ 256.612371] [drm:intel_dump_pipe_config] CURSOR PLANE:20 plane: 0.1 idx: 1 disabled, scaler_id = 0 [ 256.612372] [drm:intel_dump_pipe_config] STANDARD PLANE:22 plane: 0.1 idx: 2 disabled, scaler_id = 0 [ 256.612425] [drm:edp_panel_on] Turn eDP port A panel power on [ 256.612429] [drm:wait_panel_power_cycle] Wait for panel power cycle [ 256.612433] [drm:wait_panel_status] mask b80f value status control abcd [ 256.612436] [drm:wait_panel_status] Wait complete [ 256.612441] [drm:wait_panel_on] Wait for panel power on [ 256.612445] [drm:wait_panel_status] mask b00f value 8008 status 000a control abcd0003 [ 256.820170] [drm:wait_panel_status] Wait complete [ 256.820188] [drm:edp_panel_vdd_on] Turning eDP port A VDD on [ 256.820198] [drm:edp_panel_vdd_on] PP_STATUS: 0x8008 PP_CO
[Intel-gfx] [PATCH] drm/i915/gen9: Implement WaDisableKillLogic for gen 9
v2: Patch leakage fixed Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 4 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 059de0f..afa8972 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -160,6 +160,7 @@ #define GAM_ECOCHK 0x4090 #define BDW_DISABLE_HDC_INVALIDATION (1<<25) #define ECOCHK_SNB_BIT (1<<10) +#define ECOCHK_DIS_TLB (1<<8) #define HSW_ECOCHK_ARB_PRIO_SOL (1<<6) #define ECOCHK_PPGTT_CACHE64B(0x3<<3) #define ECOCHK_PPGTT_CACHE4B (0x0<<3) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f2be1ce..6986342 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -59,6 +59,10 @@ static void gen9_init_clock_gating(struct drm_device *dev) /* WaEnableLbsSlaRetryTimerDecrement:skl */ I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); + + /* WaDisableKillLogic:bxt,skl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + ECOCHK_DIS_TLB); } static void skl_init_clock_gating(struct drm_device *dev) -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3.1 11/20] drm/i915: Readout initial hw mode.
drm/i915: Readout initial hw mode, v2.
Atomic requires a mode blob when crtc_state->enable is true, or
you get a huge warn_on.
With a few tweaks the mode we read out from hardware could be used
as the real mode without a modeset, but this requires too much
testing, so for now force a modeset the first time the mode blob's
updated.
This preserves the old behavior, because previously we never set
the initial mode, which always meant that a modeset happened
when the mode was first set.
Changes since v1:
- Add a description in intel_modeset_readout_hw_state of how the
recalculation is done.
Signed-off-by: Maarten Lankhorst
---
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6ef3ef4f7be0..a815db672a8a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13152,7 +13152,7 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
for_each_crtc_in_state(state, crtc, crtc_state, i) {
struct intel_crtc_state *pipe_config =
to_intel_crtc_state(crtc_state);
- bool modeset, recalc;
+ bool modeset, recalc = false;
if (!crtc_state->enable) {
if (needs_modeset(crtc_state))
@@ -13161,7 +13161,10 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
}
modeset = needs_modeset(crtc_state);
- recalc = pipe_config->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE;
+ /* see comment in intel_modeset_readout_hw_state */
+ if (!modeset && crtc_state->mode_blob != crtc->state->mode_blob
&&
+ pipe_config->quirks & PIPE_CONFIG_QUIRK_INHERITED_MODE)
+ recalc = true;
if (!modeset && !recalc)
continue;
@@ -13176,9 +13179,10 @@ intel_modeset_compute_config(struct drm_atomic_state
*state)
if (ret)
return ret;
- if (recalc && !intel_pipe_config_compare(state->dev,
+ if (recalc && (!i915.fastboot ||
+ !intel_pipe_config_compare(state->dev,
to_intel_crtc_state(crtc->state),
- pipe_config, true)) {
+ pipe_config, true))) {
modeset = crtc_state->mode_changed = true;
ret = drm_atomic_add_affected_planes(state, crtc);
@@ -15457,11 +15461,42 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
crtc->active = dev_priv->display.get_pipe_config(crtc,
crtc->config);
- 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;
+ memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
+ if (crtc->base.state->active) {
+ intel_mode_from_pipe_config(&crtc->base.mode,
crtc->config);
+
intel_mode_from_pipe_config(&crtc->base.state->adjusted_mode, crtc->config);
+ WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state,
&crtc->base.mode));
+
+ /*
+* The initial mode needs to be set in order to keep
+* the atomic core happy. It wants a valid mode if the
+* crtc's enabled, so we do the above call.
+*
+* At this point some state updated by the connectors
+* in their ->detect() callback has not run yet, so
+* no recalculation can be done yet.
+*
+* Even if we could do a recalculation and modeset
+* right now it would cause a double modeset if
+* fbdev or userspace chooses a different initial mode.
+*
+* So to prevent the double modeset, fail the memcmp
+* test in drm_atomic_set_mode_for_crtc to get a new
+* mode blob, and compare if the mode blob changed
+* when the PIPE_CONFIG_QUIRK_INHERITED_MODE quirk is
+* set.
+*
+* If that happens, someone indicated they wanted a
+* mode change, which means it's safe to do a full
+* recalculation.
+*/
+ crtc->base.state->mode.private_flags = ~0;
+ }
+
+ crtc->base.hwmode = crtc->config->base.adjusted_mode;
[Intel-gfx] [PATCH v3 3/4] drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround
In Indirect context w/a batch buffer,
+WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt
v2: address static checker warning where unsigned value was checked for
less than zero which is never true (Dan Carpenter).
v3: The WA uses default value of GEN8_L3SQCREG4 during flush but that disables
some other WA; update default value to retain it and document dependency (Mika).
Cc: Mika Kuoppala
Cc: Imre Deak
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_lrc.c | 16
drivers/gpu/drm/i915/intel_pm.c | 3 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 997212b..ebe8eb1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1097,6 +1097,15 @@ static inline int gen8_emit_flush_coherentl3_wa(struct
intel_engine_cs *ring,
{
uint32_t l3sqc4_flush = (0x4040 | GEN8_LQSC_FLUSH_COHERENT_LINES);
+ /*
+* WaDisableLSQCROPERFforOCL:skl
+* This WA is implemented in skl_init_clock_gating() but since
+* this batch updates GEN8_L3SQCREG4 with default value we need to
+* set this bit here to retain the WA during flush.
+*/
+ if (IS_SKYLAKE(ring->dev) && INTEL_REVID(ring->dev) <= SKL_REVID_E0)
+ l3sqc4_flush |= GEN8_LQSC_RO_PERF_DIS;
+
wa_ctx_emit(batch, index, (MI_STORE_REGISTER_MEM_GEN8(1) |
MI_SRM_LRM_GLOBAL_GTT));
wa_ctx_emit(batch, index, GEN8_L3SQCREG4);
@@ -1253,6 +1262,7 @@ static int gen9_init_indirectctx_bb(struct
intel_engine_cs *ring,
uint32_t *const batch,
uint32_t *offset)
{
+ int ret;
struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
@@ -1261,6 +1271,12 @@ static int gen9_init_indirectctx_bb(struct
intel_engine_cs *ring,
(IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_DISABLE);
+ /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
+ ret = gen8_emit_flush_coherentl3_wa(ring, batch, index);
+ if (ret < 0)
+ return ret;
+ index = ret;
+
/* Pad to end of cacheline */
while (index % CACHELINE_DWORDS)
wa_ctx_emit(batch, index, MI_NOOP);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4e24d2b..95d06a0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -91,6 +91,9 @@ static void skl_init_clock_gating(struct drm_device *dev)
_MASKED_BIT_ENABLE(GEN9_TSG_BARRIER_ACK_DISABLE));
}
+ /* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes
+* involving this register should also be added to WA batch as required.
+*/
if (INTEL_REVID(dev) <= SKL_REVID_E0)
/* WaDisableLSQCROPERFforOCL:skl */
I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 0/4] Gen9 WA Batch patches
First two patches received r-b tags. All patches use updated macro, patch 3 and 4 updated as per review comments. v2 review history is available at, http://www.spinics.net/lists/intel-gfx/msg70952.html Arun Siluvery (4): drm/i915: Enable WA batch buffers for Gen9 drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken drivers/gpu/drm/i915/intel_lrc.c| 85 +++-- drivers/gpu/drm/i915/intel_pm.c | 3 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++- 3 files changed, 90 insertions(+), 5 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
In Indirect context w/a batch buffer,
+WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).
v3: explain why part of the WA is in Per ctx batch (Mika)
Cc: Mika Kuoppala
Cc: Imre Deak
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_lrc.c| 10 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ebe8eb1..8e2fd2e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1292,6 +1292,16 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
*ring,
struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+ /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
+ if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
+ (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
+ wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
+ wa_ctx_emit(batch, index, GEN9_SLICE_COMMON_ECO_CHICKEN0);
+ wa_ctx_emit(batch, index,
+ _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
+ wa_ctx_emit(batch, index, MI_NOOP);
+ }
+
/* WaDisableCtxRestoreArbitration:skl,bxt */
if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
(IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 385859e..177f7ed 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
/*
WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
GEN9_RHWO_OPTIMIZATION_DISABLE);
- WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
- DISABLE_PIXEL_MASK_CAMMING);
+ /*
+* WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14:14] to be
set
+* but we do that in per ctx batchbuffer as there is an issue
+* with this register not getting restored on ctx restore
+*/
}
if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 1/4] drm/i915: Enable WA batch buffers for Gen9
This patch only enables support for Gen9, the actual WA will be
initialized in subsequent patches.
The WARN that we use to warn user if WA batch support is not available
for a particular Gen is replaced with DRM_ERROR as warning here doesn't
really add much value.
v2: include all infrastructure bits in this patch so that subsequent
changes only correspond the WA added (Chris)
v3: use updated macro.
Reviewed-by: Mika Kuoppala
Cc: Imre Deak
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_lrc.c | 50 +---
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0007d45..1649830 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1248,6 +1248,35 @@ static int gen8_init_perctx_bb(struct intel_engine_cs
*ring,
return wa_ctx_end(wa_ctx, *offset = index, 1);
}
+static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
+ struct i915_wa_ctx_bb *wa_ctx,
+ uint32_t *const batch,
+ uint32_t *offset)
+{
+ uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+
+ /* FIXME: Replace me with WA */
+ wa_ctx_emit(batch, index, MI_NOOP);
+
+ /* Pad to end of cacheline */
+ while (index % CACHELINE_DWORDS)
+ wa_ctx_emit(batch, index, MI_NOOP);
+
+ return wa_ctx_end(wa_ctx, *offset = index, CACHELINE_DWORDS);
+}
+
+static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
+ struct i915_wa_ctx_bb *wa_ctx,
+ uint32_t *const batch,
+ uint32_t *offset)
+{
+ uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+
+ wa_ctx_emit(batch, index, MI_BATCH_BUFFER_END);
+
+ return wa_ctx_end(wa_ctx, *offset = index, 1);
+}
+
static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
{
int ret;
@@ -1289,10 +1318,11 @@ static int intel_init_workaround_bb(struct
intel_engine_cs *ring)
WARN_ON(ring->id != RCS);
/* update this when WA for higher Gen are added */
- if (WARN(INTEL_INFO(ring->dev)->gen > 8,
-"WA batch buffer is not initialized for Gen%d\n",
-INTEL_INFO(ring->dev)->gen))
+ if (INTEL_INFO(ring->dev)->gen > 9) {
+ DRM_ERROR("WA batch buffer is not initialized for Gen%d\n",
+ INTEL_INFO(ring->dev)->gen);
return 0;
+ }
/* some WA perform writes to scratch page, ensure it is valid */
if (ring->scratch.obj == NULL) {
@@ -1324,6 +1354,20 @@ static int intel_init_workaround_bb(struct
intel_engine_cs *ring)
&offset);
if (ret)
goto out;
+ } else if (INTEL_INFO(ring->dev)->gen == 9) {
+ ret = gen9_init_indirectctx_bb(ring,
+ &wa_ctx->indirect_ctx,
+ batch,
+ &offset);
+ if (ret)
+ goto out;
+
+ ret = gen9_init_perctx_bb(ring,
+ &wa_ctx->per_ctx,
+ batch,
+ &offset);
+ if (ret)
+ goto out;
}
out:
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround
In Indirect and Per context w/a batch buffer,
+WaDisableCtxRestoreArbitration
v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).
v3: use updated macro.
Reviewed-by: Mika Kuoppala
Cc: Robert Beckett
Cc: Mika Kuoppala
Cc: Imre Deak
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/intel_lrc.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1649830..997212b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1253,10 +1253,13 @@ static int gen9_init_indirectctx_bb(struct
intel_engine_cs *ring,
uint32_t *const batch,
uint32_t *offset)
{
+ struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
- /* FIXME: Replace me with WA */
- wa_ctx_emit(batch, index, MI_NOOP);
+ /* WaDisableCtxRestoreArbitration:skl,bxt */
+ if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+ (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
+ wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_DISABLE);
/* Pad to end of cacheline */
while (index % CACHELINE_DWORDS)
@@ -1270,8 +1273,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
*ring,
uint32_t *const batch,
uint32_t *offset)
{
+ struct drm_device *dev = ring->dev;
uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+ /* WaDisableCtxRestoreArbitration:skl,bxt */
+ if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+ (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
+ wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_ENABLE);
+
wa_ctx_emit(batch, index, MI_BATCH_BUFFER_END);
return wa_ctx_end(wa_ctx, *offset = index, 1);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3.1 15/20] drm/i915: Always force a modeset in intel_crtc_restore_mode, v2.
And get rid of things that are no longer true. This function is only
used for forcing a modeset when encoder properties are changed.
Because this is not yet done atomically, assume a full modeset is
needed and force a modeset on the crtc.
Changes since v1:
- s/reset/force modeset/
Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Stone
---
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 9537ad8be4a9..499223e6f736 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13273,63 +13273,37 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
struct drm_atomic_state *state;
- struct intel_encoder *encoder;
- struct intel_connector *connector;
- struct drm_connector_state *connector_state;
- struct intel_crtc_state *crtc_state;
+ struct drm_crtc_state *crtc_state;
int ret;
state = drm_atomic_state_alloc(dev);
if (!state) {
- DRM_DEBUG_KMS("[CRTC:%d] mode restore failed, out of memory",
+ DRM_DEBUG_KMS("[CRTC:%d] crtc restore failed, out of memory",
crtc->base.id);
return;
}
- state->acquire_ctx = dev->mode_config.acquire_ctx;
-
- /* The force restore path in the HW readout code relies on the staged
-* config still keeping the user requested config while the actual
-* state has been overwritten by the configuration read from HW. We
-* need to copy the staged config to the atomic state, otherwise the
-* mode set will just reapply the state the HW is already in. */
- for_each_intel_encoder(dev, encoder) {
- if (encoder->base.crtc != crtc)
- continue;
+ state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
- for_each_intel_connector(dev, connector) {
- if (connector->base.state->best_encoder !=
- &encoder->base)
- continue;
-
- connector_state = drm_atomic_get_connector_state(state,
&connector->base);
- if (IS_ERR(connector_state)) {
- DRM_DEBUG_KMS("Failed to add [CONNECTOR:%d:%s]
to state: %ld\n",
- connector->base.base.id,
- connector->base.name,
- PTR_ERR(connector_state));
- continue;
- }
+retry:
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ ret = PTR_ERR_OR_ZERO(crtc_state);
+ if (!ret) {
+ if (!crtc_state->active)
+ goto out;
- connector_state->crtc = crtc;
- }
+ crtc_state->mode_changed = true;
+ ret = intel_set_mode(state);
}
- 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));
- drm_atomic_state_free(state);
- return;
+ if (ret == -EDEADLK) {
+ drm_atomic_state_clear(state);
+ drm_modeset_backoff(state->acquire_ctx);
+ goto retry;
}
- 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);
-
- ret = intel_set_mode(state);
if (ret)
+out:
drm_atomic_state_free(state);
}
___
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/bxt: WA for swapped HPD pins in A stepping
On ti, 2015-07-14 at 11:18 +0530, Sonika Jindal wrote:
> As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
> and interrupts to check the external panel connection and DDIC HPD
> logic for edp panel.
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 --
> drivers/gpu/drm/i915/intel_hdmi.c |9 -
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7b54b9d..c32f3d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5869,10 +5869,24 @@ intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
> /* Set up the hotplug pin. */
> switch (port) {
> case PORT_A:
> - intel_encoder->hpd_pin = HPD_PORT_A;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIC HPD logic and
> + * interrupts to check the eDP panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_B:
> - intel_encoder->hpd_pin = HPD_PORT_B;
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + else
> + intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
This won't work for a couple of reasons: atm i915_digport_work_func()
assumes a fixed pin->port mapping, for example it'll call the HPD
handler for the port A encoder for a short/long pulse event triggered
via the HPD_PORT_A pin, whereas after the above patch you want to select
port B's encoder on BXT A0/1. This could be fixed by setting up
hotplug.irq_port in intel_ddi_init() according to the above change, or
not using irq_port at all, but instead looking up the encoder the same
way i915_hotplug_work_func() does using intel_encoder->hpd_pin.
The HPD_PORT_A HPD handling is missing in general, see
for_each_hpd_pin() and intel_hpd_irq_handler()/is_dig_port.
So if going with the above way, these issues need to be addressed first.
--Imre
___
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: Check live status before reading edid
On ti, 2015-07-14 at 17:21 +0530, Sonika Jindal wrote:
> Adding this for SKL onwards.
>
> v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
> to check digital port status. Adding a separate function to get bxt live
> status (Daniel)
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_dp.c |4 ++--
> drivers/gpu/drm/i915/intel_drv.h |2 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 43
> +
> 3 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4ebfc3a..7b54b9d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4443,8 +4443,8 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
> return intel_dp_detect_dpcd(intel_dp);
> }
>
> -static int g4x_digital_port_connected(struct drm_device *dev,
> -struct intel_digital_port
> *intel_dig_port)
> +int g4x_digital_port_connected(struct drm_device *dev,
> +struct intel_digital_port *intel_dig_port)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> uint32_t bit;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a47fbc3..30c8dd6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1187,6 +1187,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
> void intel_edp_drrs_invalidate(struct drm_device *dev,
> unsigned frontbuffer_bits);
> void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
> +int g4x_digital_port_connected(struct drm_device *dev,
> +struct intel_digital_port *intel_dig_port);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int
> conn_id);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index c4b82ce..402be54 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1300,6 +1300,40 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
> to_intel_connector(connector)->detect_edid = NULL;
> }
>
> +static bool bxt_port_connected(struct drm_i915_private *dev_priv,
> +struct intel_digital_port *port)
> +{
> + u32 temp = I915_READ(GEN8_DE_PORT_ISR);
> +
> + /* TODO: Add bxt A0/A1 wa related to hpd pin swap */
This doesn't seem to work on my BXT A1 RVP. For me GEN8_DE_PORT_ISR
reads all 0 even while an HDMI monitor is plugged to port B.
> + switch (port->port) {
> + case PORT_B:
> + return temp & BXT_DE_PORT_HP_DDIB;
> +
> + case PORT_C:
> + return temp & BXT_DE_PORT_HP_DDIC;
> +
> + default:
> + return false;
> +
> + }
> +}
> +
> +static bool intel_hdmi_live_status(struct intel_digital_port *intel_dig_port)
> +{
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> +
> + if (IS_VALLEYVIEW(dev))
> + return g4x_digital_port_connected(dev, intel_dig_port);
> + else if (IS_SKYLAKE(dev))
> + return ibx_digital_port_connected(dev_priv, intel_dig_port);
> + else if (IS_BROXTON(dev))
> + return bxt_port_connected(dev_priv, intel_dig_port);
> +
> + return true;
> +}
> +
> static bool
> intel_hdmi_set_edid(struct drm_connector *connector)
> {
> @@ -1308,15 +1342,16 @@ intel_hdmi_set_edid(struct drm_connector *connector)
> struct intel_encoder *intel_encoder =
> &hdmi_to_dig_port(intel_hdmi)->base;
> enum intel_display_power_domain power_domain;
> - struct edid *edid;
> + struct edid *edid = NULL;
> bool connected = false;
>
> power_domain = intel_display_port_power_domain(intel_encoder);
> intel_display_power_get(dev_priv, power_domain);
>
> - edid = drm_get_edid(connector,
> - intel_gmbus_get_adapter(dev_priv,
> - intel_hdmi->ddc_bus));
> + if (intel_hdmi_live_status(hdmi_to_dig_port(intel_hdmi)))
> + edid = drm_get_edid(connector,
> + intel_gmbus_get_adapter(dev_priv,
> + intel_hdmi->ddc_bus));
>
> intel_display_power_put(dev_priv, power_domain);
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv9] drm/i915: Added Programming of the MOCS
On Fri, Jul 10, 2015 at 08:13:11PM +0300, Francisco Jerez wrote: > From: Peter Antoine > > This change adds the programming of the MOCS registers to the gen 9+ > platforms. The set of MOCS configuration entries introduced by this > patch is intended to be minimal but sufficient to cover the needs of > current userspace - i.e. a good set of defaults. It is expected to be > extended in the future to provide further default values or to allow > userspace to redefine its private MOCS tables based on its demand for > additional caching configurations. In this setup, userspace should > only utilize the first N entries, higher entries are reserved for > future use. > > It creates a fixed register set that is programmed across the different > engines so that all engines have the same table. This is done as the > main RCS context only holds the registers for itself and the shared > L3 values. By trying to keep the registers consistent across the > different engines it should make the programming for the registers > consistent. > > v2: > -'static const' for private data structures and style changes.(Matt Turner) > v3: > - Make the tables "slightly" more readable. (Damien Lespiau) > - Updated tables fix performance regression. > v4: > - Code formatting. (Chris Wilson) > - re-privatised mocs code. (Daniel Vetter) > v5: > - Changed the name of a function. (Chris Wilson) > v6: > - re-based > - Added Mesa table entry (skylake & broxton) (Francisco Jerez) > - Tidied up the readability defines (Francisco Jerez) > - NUMBER of entries defines wrong. (Jim Bish) > - Added comments to clear up the meaning of the tables (Jim Bish) > > Signed-off-by: Peter Antoine > > v7 (Francisco Jerez): > - Don't write L3-specific MOCS_ESC/SCC values into the e/LLC control > tables. Prefix L3-specific defines consistently with L3_ and > e/LLC-specific defines with LE_ to avoid this kind of confusion in > the future. > - Change L3CC WT define back to RESERVED (matches my hardware > documentation and the original patch, probably a misunderstanding > of my own previous comment). > - Drop Android tables, define new minimal tables more suitable for the > open source stack. > - Add comment that the MOCS tables are part of the kernel ABI. > - Move intel_logical_ring_begin() and _advance() calls one level down > (Chris Wilson). > - Minor formatting and style fixes. > v8 (Francisco Jerez): > - Add table size sanity check to emit_mocs_control/l3cc_table() (Chris > Wilson). > - Add comment about undefined entries being implicitly set to uncached > for forwards compatibility. > v9 (Francisco Jerez): > - Minor style fixes. What's happening here? are we ready to commit to this ABI? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCHv9] drm/i915: Added Programming of the MOCS
Damien Lespiau writes: > On Fri, Jul 10, 2015 at 08:13:11PM +0300, Francisco Jerez wrote: >> From: Peter Antoine >> >> This change adds the programming of the MOCS registers to the gen 9+ >> platforms. The set of MOCS configuration entries introduced by this >> patch is intended to be minimal but sufficient to cover the needs of >> current userspace - i.e. a good set of defaults. It is expected to be >> extended in the future to provide further default values or to allow >> userspace to redefine its private MOCS tables based on its demand for >> additional caching configurations. In this setup, userspace should >> only utilize the first N entries, higher entries are reserved for >> future use. >> >> It creates a fixed register set that is programmed across the different >> engines so that all engines have the same table. This is done as the >> main RCS context only holds the registers for itself and the shared >> L3 values. By trying to keep the registers consistent across the >> different engines it should make the programming for the registers >> consistent. >> >> v2: >> -'static const' for private data structures and style changes.(Matt Turner) >> v3: >> - Make the tables "slightly" more readable. (Damien Lespiau) >> - Updated tables fix performance regression. >> v4: >> - Code formatting. (Chris Wilson) >> - re-privatised mocs code. (Daniel Vetter) >> v5: >> - Changed the name of a function. (Chris Wilson) >> v6: >> - re-based >> - Added Mesa table entry (skylake & broxton) (Francisco Jerez) >> - Tidied up the readability defines (Francisco Jerez) >> - NUMBER of entries defines wrong. (Jim Bish) >> - Added comments to clear up the meaning of the tables (Jim Bish) >> >> Signed-off-by: Peter Antoine >> >> v7 (Francisco Jerez): >> - Don't write L3-specific MOCS_ESC/SCC values into the e/LLC control >> tables. Prefix L3-specific defines consistently with L3_ and >> e/LLC-specific defines with LE_ to avoid this kind of confusion in >> the future. >> - Change L3CC WT define back to RESERVED (matches my hardware >> documentation and the original patch, probably a misunderstanding >> of my own previous comment). >> - Drop Android tables, define new minimal tables more suitable for the >> open source stack. >> - Add comment that the MOCS tables are part of the kernel ABI. >> - Move intel_logical_ring_begin() and _advance() calls one level down >> (Chris Wilson). >> - Minor formatting and style fixes. >> v8 (Francisco Jerez): >> - Add table size sanity check to emit_mocs_control/l3cc_table() (Chris >> Wilson). >> - Add comment about undefined entries being implicitly set to uncached >> for forwards compatibility. >> v9 (Francisco Jerez): >> - Minor style fixes. > > What's happening here? are we ready to commit to this ABI? I'm for it. I also sent a patch for userspace to switch to the new tables [1] and already have an R-b on it. [1] http://lists.freedesktop.org/archives/mesa-dev/2015-July/088310.html > > -- > Damien signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [-next] WARNING at i915_gem_track_fb
On Tue, Jul 14, 2015 at 10:41:42PM +0900, Sergey Senozhatsky wrote: > On (07/14/15 14:44), Daniel Vetter wrote: > > > that helped. seems to be working only on -next. > > > > You mean you only get a backtrace on -next, right? > > yeah, sure :-) > > > Otherwise I'd be confused ;-) > > > > Next up. Please boot with drm.debug=0xe, repro the issue and attach > > complete dmesg (from boot-up up to the WARNING). That should help us > > reconstruct how things went wrong here. > > can't reproduce it thus far. Have you forwarded to a more recent -nightly? I just merged a patch which might have fixed this ... -Daniel > > sometimes `xset dpms force off' just turns off the panel for a second, > sometimes -- until I generate a `wakeup' event (key press, etc.) > part of dmesg (no WARNING yet) > > [ 253.699215] [drm:check_encoder_state] [ENCODER:32:DAC-32] > [ 253.699217] [drm:check_encoder_state] [ENCODER:33:TMDS-33] > [ 253.699219] [drm:check_encoder_state] [ENCODER:41:TMDS-41] > [ 253.699221] [drm:check_crtc_state] [CRTC:21] > [ 253.699225] [drm:check_crtc_state] [CRTC:25] > [ 253.699226] [drm:check_crtc_state] [CRTC:29] > [ 253.699228] [drm:check_shared_dpll_state] WRPLL 1 > [ 253.699230] [drm:check_shared_dpll_state] WRPLL 2 > [ 253.699270] [drm:intel_backlight_device_update_status] updating > intel_backlight, brightness=0/976 > [ 256.612288] [drm:drm_mode_setcrtc] [CRTC:21] > [ 256.612299] [drm:drm_mode_setcrtc] [CONNECTOR:34:eDP-1] > [ 256.612302] [drm:intel_crtc_set_config] [CRTC:21] [FB:45] #connectors=1 (x > y) (0 0) > [ 256.612308] [drm:intel_modeset_stage_output_state] [CONNECTOR:34:eDP-1] to > [CRTC:21] > [ 256.612317] [drm:connected_sink_compute_bpp] [CONNECTOR:34:eDP-1] checking > for sink bpp constrains > [ 256.612318] [drm:connected_sink_compute_bpp] clamping display bpp (was 36) > to EDID reported max of 18 > [ 256.612322] [drm:intel_dp_compute_config] DP link computation with max > lane count 2 max bw 27 pixel clock 143000KHz > [ 256.612324] [drm:intel_dp_compute_config] DP link bw 0a lane count 2 clock > 27 bpp 18 > [ 256.612326] [drm:intel_dp_compute_config] DP link bw required 257400 > available 432000 > [ 256.612328] [drm:intel_modeset_pipe_config] plane bpp: 36, pipe bpp: 18, > dithering: 1 > [ 256.612331] [drm:intel_dump_pipe_config] [CRTC:21][modeset] config > 8800c6683400 for pipe A > [ 256.612332] [drm:intel_dump_pipe_config] cpu_transcoder: D > [ 256.612333] [drm:intel_dump_pipe_config] pipe bpp: 18, dithering: 1 > [ 256.612335] [drm:intel_dump_pipe_config] fdi/pch: 0, lanes: 0, gmch_m: 0, > gmch_n: 0, link_m: 0, link_n: 0, tu: 0 > [ 256.612337] [drm:intel_dump_pipe_config] dp: 1, gmch_m: 4998212, gmch_n: > 8388608, link_m: 277678, link_n: 524288, tu: 64 > [ 256.612339] [drm:intel_dump_pipe_config] dp: 1, gmch_m2: 0, gmch_n2: 0, > link_m2: 0, link_n2: 0, tu2: 0 > [ 256.612340] [drm:intel_dump_pipe_config] audio: 0, infoframes: 0 > [ 256.612342] [drm:intel_dump_pipe_config] requested mode: > [ 256.612344] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 60 > 143000 1920 1968 2000 2080 1080 1082 1087 1144 0x0 0xa > [ 256.612346] [drm:intel_dump_pipe_config] adjusted mode: > [ 256.612348] [drm:drm_mode_debug_printmodeline] Modeline 0:"1920x1080" 60 > 143000 1920 1968 2000 2080 1080 1082 1087 1144 0x48 0xa > [ 256.612350] [drm:intel_dump_crtc_timings] crtc timings: 143000 1920 1968 > 2000 2080 1080 1082 1087 1144, type: 0x48 flags: 0xa > [ 256.612351] [drm:intel_dump_pipe_config] port clock: 27 > [ 256.612353] [drm:intel_dump_pipe_config] pipe src size: 1920x1080 > [ 256.612354] [drm:intel_dump_pipe_config] num_scalers: 0, scaler_users: > 0x0, scaler_id: 0 > [ 256.612356] [drm:intel_dump_pipe_config] gmch pfit: control: 0x, > ratios: 0x, lvds border: 0x > [ 256.612358] [drm:intel_dump_pipe_config] pch pfit: pos: 0x, size: > 0x, disabled > [ 256.612359] [drm:intel_dump_pipe_config] ips: 0 > [ 256.612360] [drm:intel_dump_pipe_config] double wide: 0 > [ 256.612362] [drm:intel_dump_pipe_config] ddi_pll_sel: 536870912; > dpll_hw_state: wrpll: 0x0 > [ 256.612363] [drm:intel_dump_pipe_config] planes on this crtc > [ 256.612365] [drm:intel_dump_pipe_config] STANDARD PLANE:18 plane: 0.0 idx: > 0 enabled > [ 256.612367] [drm:intel_dump_pipe_config] FB:45, fb = 1920x1080 format = > 0x34325258 > [ 256.612369] [drm:intel_dump_pipe_config] scaler:0 src (0, 0) 0x0 dst (0, > 0) 0x0 > [ 256.612371] [drm:intel_dump_pipe_config] CURSOR PLANE:20 plane: 0.1 idx: 1 > disabled, scaler_id = 0 > [ 256.612372] [drm:intel_dump_pipe_config] STANDARD PLANE:22 plane: 0.1 idx: > 2 disabled, scaler_id = 0 > [ 256.612425] [drm:edp_panel_on] Turn eDP port A panel power on > [ 256.612429] [drm:wait_panel_power_cycle] Wait for panel power cycle > [ 256.612433] [drm:wait_panel_status] mask b80f value status > control abcd > [ 256.612436] [drm:wait_panel_sta
Re: [Intel-gfx] [PATCHv9] drm/i915: Added Programming of the MOCS
On Tue, Jul 14, 2015 at 05:47:37PM +0300, Francisco Jerez wrote: > Damien Lespiau writes: > > > On Fri, Jul 10, 2015 at 08:13:11PM +0300, Francisco Jerez wrote: > >> From: Peter Antoine > >> > >> This change adds the programming of the MOCS registers to the gen 9+ > >> platforms. The set of MOCS configuration entries introduced by this > >> patch is intended to be minimal but sufficient to cover the needs of > >> current userspace - i.e. a good set of defaults. It is expected to be > >> extended in the future to provide further default values or to allow > >> userspace to redefine its private MOCS tables based on its demand for > >> additional caching configurations. In this setup, userspace should > >> only utilize the first N entries, higher entries are reserved for > >> future use. > >> > >> It creates a fixed register set that is programmed across the different > >> engines so that all engines have the same table. This is done as the > >> main RCS context only holds the registers for itself and the shared > >> L3 values. By trying to keep the registers consistent across the > >> different engines it should make the programming for the registers > >> consistent. > >> > >> v2: > >> -'static const' for private data structures and style changes.(Matt Turner) > >> v3: > >> - Make the tables "slightly" more readable. (Damien Lespiau) > >> - Updated tables fix performance regression. > >> v4: > >> - Code formatting. (Chris Wilson) > >> - re-privatised mocs code. (Daniel Vetter) > >> v5: > >> - Changed the name of a function. (Chris Wilson) > >> v6: > >> - re-based > >> - Added Mesa table entry (skylake & broxton) (Francisco Jerez) > >> - Tidied up the readability defines (Francisco Jerez) > >> - NUMBER of entries defines wrong. (Jim Bish) > >> - Added comments to clear up the meaning of the tables (Jim Bish) > >> > >> Signed-off-by: Peter Antoine > >> > >> v7 (Francisco Jerez): > >> - Don't write L3-specific MOCS_ESC/SCC values into the e/LLC control > >> tables. Prefix L3-specific defines consistently with L3_ and > >> e/LLC-specific defines with LE_ to avoid this kind of confusion in > >> the future. > >> - Change L3CC WT define back to RESERVED (matches my hardware > >> documentation and the original patch, probably a misunderstanding > >> of my own previous comment). > >> - Drop Android tables, define new minimal tables more suitable for the > >> open source stack. > >> - Add comment that the MOCS tables are part of the kernel ABI. > >> - Move intel_logical_ring_begin() and _advance() calls one level down > >> (Chris Wilson). > >> - Minor formatting and style fixes. > >> v8 (Francisco Jerez): > >> - Add table size sanity check to emit_mocs_control/l3cc_table() (Chris > >> Wilson). > >> - Add comment about undefined entries being implicitly set to uncached > >> for forwards compatibility. > >> v9 (Francisco Jerez): > >> - Minor style fixes. > > > > What's happening here? are we ready to commit to this ABI? > > I'm for it. I also sent a patch for userspace to switch to the new > tables [1] and already have an R-b on it. > > [1] http://lists.freedesktop.org/archives/mesa-dev/2015-July/088310.html Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
Arun Siluvery writes:
> In Indirect context w/a batch buffer,
> +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
>
> v2: SKL revision id was used for BXT, copy paste error found during
> internal review (Bob Beckett).
>
> v3: explain why part of the WA is in Per ctx batch (Mika)
>
> Cc: Mika Kuoppala
> Cc: Imre Deak
> Signed-off-by: Arun Siluvery
Patches 3/4 and 4/4,
Reviewed-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_lrc.c| 10 ++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index ebe8eb1..8e2fd2e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1292,6 +1292,16 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
> *ring,
> struct drm_device *dev = ring->dev;
> uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>
> + /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
> + if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
> + (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
> + wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
> + wa_ctx_emit(batch, index, GEN9_SLICE_COMMON_ECO_CHICKEN0);
> + wa_ctx_emit(batch, index,
> + _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
> + wa_ctx_emit(batch, index, MI_NOOP);
> + }
> +
> /* WaDisableCtxRestoreArbitration:skl,bxt */
> if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
> (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 385859e..177f7ed 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs
> *ring)
> /*
> WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
> WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
> GEN9_RHWO_OPTIMIZATION_DISABLE);
> - WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
> - DISABLE_PIXEL_MASK_CAMMING);
> + /*
> + * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14:14] to be
> set
> + * but we do that in per ctx batchbuffer as there is an issue
> + * with this register not getting restored on ctx restore
> + */
> }
>
> if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
> --
> 1.9.1
>
> ___
> 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
[Intel-gfx] [PATCH] drm/i915: Use expcitly fixed type in compat32 structs
I was confused shortly whether the compat was needed for the int,
until I noticed the pointer in the original.
Also remove typedef.
v2: Review from Chris.
- Add comments.
- Also change the int param in the original structure.
Cc: Chris Wilson
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_ioc32.c | 13 +
include/uapi/drm/i915_drm.h | 6 +-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_ioc32.c
b/drivers/gpu/drm/i915/i915_ioc32.c
index 6eec2221b44e..a5a2d5b3f44b 100644
--- a/drivers/gpu/drm/i915/i915_ioc32.c
+++ b/drivers/gpu/drm/i915/i915_ioc32.c
@@ -35,15 +35,20 @@
#include
#include "i915_drv.h"
-typedef struct drm_i915_getparam32 {
- int param;
+struct drm_i915_getparam32 {
+ s32 param;
+ /*
+* We screwed up the generic ioctl struct here and used a variable-sized
+* pointer. Use u32 in the compat struct to match the 32bit pointer
+* userspace expects.
+*/
u32 value;
-} drm_i915_getparam32_t;
+};
static int compat_i915_getparam(struct file *file, unsigned int cmd,
unsigned long arg)
{
- drm_i915_getparam32_t req32;
+ struct drm_i915_getparam32 req32;
drm_i915_getparam_t __user *request;
if (copy_from_user(&req32, (void __user *)arg, sizeof(req32)))
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e7c29f1659ad..192027b4f031 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -358,7 +358,11 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_HAS_RESOURCE_STREAMER 36
typedef struct drm_i915_getparam {
- int param;
+ s32 param;
+ /*
+* WARNING: Using pointers instead of fixed-size u64 means we need to
write
+* compat32 code. Don't repeat this mistake.
+*/
int __user *value;
} drm_i915_getparam_t;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use expcitly fixed type in compat32 structs
On Tue, Jul 14, 2015 at 06:07:30PM +0200, Daniel Vetter wrote: > I was confused shortly whether the compat was needed for the int, > until I noticed the pointer in the original. > > Also remove typedef. > > v2: Review from Chris. > - Add comments. > - Also change the int param in the original structure. > > Cc: Chris Wilson > Signed-off-by: Daniel Vetter I think there is much more value in documenting every mistake we make in the uabi than there is the fixes themselves, so the comments are much appreciated. 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
Re: [Intel-gfx] [PATCH v3 16/20] drm/i915: Make intel_display_suspend atomic, try 2.
On Mon, Jul 13, 2015 at 04:30:29PM +0200, Maarten Lankhorst wrote:
> Calculate all state using a normal transition, but afterwards fudge
> crtc->state->active back to its old value. This should still allow
> state restore in setup_hw_state to work properly.
>
> Signed-off-by: Maarten Lankhorst
Ok merged up to this patch with the exception of patch 8. I think
reworking that logic to use mode->private_flags to make sure we do a
modeset when userspace changes the mode, but not earlier is a sound
approach which will address all my concerns.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 52
> +---
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 2 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 599af76d34f6..6e3df10a43b9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6224,12 +6224,58 @@ static void intel_crtc_disable_noatomic(struct
> drm_crtc *crtc)
> * 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)
> +int intel_display_suspend(struct drm_device *dev)
> {
> + struct drm_mode_config *config = &dev->mode_config;
> + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
> + struct drm_atomic_state *state;
> struct drm_crtc *crtc;
> + unsigned crtc_mask = 0;
> + int ret = 0;
> +
> + if (WARN_ON(!ctx))
> + return 0;
> +
> + lockdep_assert_held(&ctx->ww_ctx);
> + state = drm_atomic_state_alloc(dev);
> + if (WARN_ON(!state))
> + return -ENOMEM;
> +
> + state->acquire_ctx = ctx;
> + state->allow_modeset = true;
> +
> + for_each_crtc(dev, crtc) {
> + struct drm_crtc_state *crtc_state =
> + drm_atomic_get_crtc_state(state, crtc);
>
> - for_each_crtc(dev, crtc)
> - intel_crtc_disable_noatomic(crtc);
> + ret = PTR_ERR_OR_ZERO(crtc_state);
> + if (ret)
> + goto free;
> +
> + if (!crtc_state->active)
> + continue;
> +
> + crtc_state->active = false;
> + crtc_mask |= 1 << drm_crtc_index(crtc);
> + }
> +
> + if (crtc_mask) {
> + ret = intel_set_mode(state);
> +
> + if (!ret) {
> + for_each_crtc(dev, crtc)
> + if (crtc_mask & (1 << drm_crtc_index(crtc)))
> + crtc->state->active = true;
> +
> + return ret;
> + }
> + }
> +
> +free:
> + if (ret)
> + DRM_ERROR("Suspending crtc's failed with %i\n", ret);
> + drm_atomic_state_free(state);
> + return ret;
> }
>
> /* Master function to enable/disable CRTC and corresponding power wells */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 217b773e0673..f4abce103221 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -988,7 +988,7 @@ int intel_pch_rawclk(struct drm_device *dev);
> void intel_mark_busy(struct drm_device *dev);
> void intel_mark_idle(struct drm_device *dev);
> void intel_crtc_restore_mode(struct drm_crtc *crtc);
> -void intel_display_suspend(struct drm_device *dev);
> +int intel_display_suspend(struct drm_device *dev);
> int intel_crtc_control(struct drm_crtc *crtc, bool enable);
> void intel_crtc_update_dpms(struct drm_crtc *crtc);
> void intel_encoder_destroy(struct drm_encoder *encoder);
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Declare the swizzling unknown for L-shaped configurations"
On Tue, Jul 14, 2015 at 12:31:21PM +0200, Daniel Vetter wrote: > This reverts commit 19ee835cdb0b5a8eb11a68f25a51b8039d564488. > > It breaks existing old userspace which doesn't handle UNKNOWN > swizzling correct. Yes UNKNOWN was a thing back in 2009 and probably > still is on some other platforms, but it still pretty clearly broke > the testers machine. If we want this we need to extend the ioctl with > new paramters that only new userspace looks at. > > Cc: Harald Arnesen > Cc: Chris Wilson > Reported-by: Harald Arnesen > Cc: sta...@vger.kernel.org > Signed-off-by: Daniel Vetter I am annoyed, but under the rules we have to revert this because userspace (me) fscked up. Acked-by: Chris Wilson But v1 should work in 99.9% of cases. -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 5/6] drm/i915: check for the supported strides on HSW+ FBC
2015-07-09 14:28 GMT-03:00 Paulo Zanoni :
> 2015-07-09 14:15 GMT-03:00 Daniel Vetter :
>> Plus igt testcases to make sure we check for this (since
>> right now it seems like we don't).
>
> It's on the TODO list but it's not a priority since the Kernel checks
> are very straightforward. One of the problems is that different
> platforms have different requirements, so I'll have to hardcode those
> requirements on IGT too. And I'll have to stop using igt_create_fb for
> that case since it only use powers of 2 for stride.
>
> So yeah, one day we may have the test, but not a priority right now
> since it's very easy to look at the Kernel code and make sure it's
> correct.
Ok, so I implemented fbc-badstride with the restrictions mentioned in
this patch:
http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/commit/?id=cb3861a9e3f1bc12765160345bb0dd1d543f5086
If we add more restrictions we'll have to update the test.
>
>> Additional checks here in the fbc code
>> don't seem required. But if you want to I guess you could convert them to
>> WARN_ON (without bailing out).
>> -Daniel
>>
>>> + (fb->pitches[0] & (64 - 1)) != 0) {
>>> + set_no_fbc_reason(dev_priv, FBC_BAD_STRIDE);
>>> + goto out_disable;
>>> + }
>>> +
>>> /* If the kernel debugger is active, always disable compression */
>>> if (in_dbg_master()) {
>>> set_no_fbc_reason(dev_priv, FBC_IN_DBG_MASTER);
>>> --
>>> 2.1.4
>>>
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
2015-07-09 14:39 GMT-03:00 Ville Syrjälä : > On Thu, Jul 09, 2015 at 02:31:15PM -0300, Paulo Zanoni wrote: >> 2015-07-09 14:22 GMT-03:00 Ville Syrjälä : >> > On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote: >> >> On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: >> >> > From: Paulo Zanoni >> >> > >> >> > The doc is pretty clear that this register should be set to 0 on SNB. >> >> > We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. >> >> > >> >> > Signed-off-by: Paulo Zanoni >> >> >> >> Hm, do we have testcases where we have a sufficiently big y offset? We can >> >> just allocate 128 lines more and use that as the offset, that should be >> >> big enough everywhere. Actually make that 129 lines to check the tile-size >> >> rounding ;-) >> >> >> >> Ofc this means we need to have two sets of testcases for all the affected >> >> tests (i.e. everything that tries to test the gtt hw tracking). >> >> >> >> Another funny corner case (which we're getting wrong on skl even without >> >> fbc) is x offsets > 2048 pixels (since x/y offset registers don't hold >> >> bigger values and then it wraps). >> >> >> >> I.e. I'd like this patch (and the others) to be augmented with a Testcase: >> >> tag. >> > >> > I think the entire Y offset thing is currently being misprogrammed. >> > IIRC the offset is from the display base address but we program in >> > the offset from the start of the FB. >> >> After patch 3, all the current tests pass on BDW. Can you suggest a >> different test that won't pass? > > Ah patch 3 tries to fix it. It's not entirely accurate though since it > simply relies on an implementation detail of intel_gen4_compute_page_offset(). > Well, assuming my recollection of the hardware details is correct. > > Also IIRC intel_gen4_compute_page_offset() isn't even used on SKL/BXT > currently, so it should fail on those platforms. Daniel clarified the problem to me, so I implemented the following test case: http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/commit/?id=04d1311fc3d2127d609b5c5e670bf9887652cb17 I hope this exercises the problem you're mentioning. So far I only tested BDW and it passes. > > -- > Ville Syrjälä > Intel OTC -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/4] FBC frontbuffer tracking + MMAP_WC fixes
From: Paulo Zanoni Hi This series can be reviewed in parallel with the other FBC patches currently under review. The goal here is to fix all the mmap-wc subtests of kms_frontbuffer_tracking that don't require any patches that are in the other series. Thanks, Paulo Paulo Zanoni (4): drm/i915: fix FBC frontbuffer tracking flushing code drm/i915: don't call intel_fbc_update() at intel_unpin_work_fn() drm/i915: don't disable FBC for pipe A when flipping pipe B drm/i915: special-case dirtyfb for frontbuffer tracking drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 7 ++- drivers/gpu/drm/i915/intel_drv.h | 2 +- drivers/gpu/drm/i915/intel_fbc.c | 13 +++-- drivers/gpu/drm/i915/intel_frontbuffer.c | 2 +- 5 files changed, 12 insertions(+), 13 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: don't call intel_fbc_update() at intel_unpin_work_fn()
From: Paulo Zanoni Because intel_unpin_work_fn() already calls intel_frontbuffer_flip_complete() which will call intel_fbc_flush() which will call intel_fbc_update() when needed. We couldn't fix this previously due to the fact that FBC was not properly behaving as intended on frontbuffer flushes, but now that this is fixed, we can remove the additional call. Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_display.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ad0fc6a..37b2528 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10765,15 +10765,12 @@ static void intel_unpin_work_fn(struct work_struct *__work) container_of(__work, struct intel_unpin_work, work); struct intel_crtc *crtc = to_intel_crtc(work->crtc); struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_plane *primary = crtc->base.primary; mutex_lock(&dev->struct_mutex); intel_unpin_fb_obj(work->old_fb, primary->state); drm_gem_object_unreference(&work->pending_flip_obj->base); - intel_fbc_update(dev_priv); - if (work->flip_queued_req) i915_gem_request_assign(&work->flip_queued_req, NULL); mutex_unlock(&dev->struct_mutex); -- 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: don't disable FBC for pipe A when flipping pipe B
From: Paulo Zanoni Use the appropriate call. I know there's a discussion about whether we need this call here at all, but removing the call means we'll only update FBC after we get the page flip IRQ. So the user may only see the new frame a little after it should. Let's wait just a little bit more before removing this call since we can rely in the HW tracking for accurate flips. Signed-off-by: Paulo Zanoni --- 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 37b2528..6e3ba5f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11544,7 +11544,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, to_intel_plane(primary)->frontbuffer_bit); mutex_unlock(&dev->struct_mutex); - intel_fbc_disable(dev_priv); + intel_fbc_disable_crtc(intel_crtc); intel_frontbuffer_flip_prepare(dev, to_intel_plane(primary)->frontbuffer_bit); -- 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: special-case dirtyfb for frontbuffer tracking
From: Paulo Zanoni
First, an introduction. We currently have two types of GTT mmaps: the
"normal" old mmap, and the WC mmap. For frontbuffer-related features
that have automatic hardware tracking, only the non-WC mmap writes are
detected by the hardware. Since inside the Kernel both are treated as
ORIGIN_GTT, any features ignoring ORIGIN_GTT because of the hardware
tracking are destined to fail.
One of the special rules defined for the WC mmaps is that the user
should call the dirtyfb IOCTL after he is done using the pointers, so
that results in an intel_fb_obj_flush() call. The problem is that the
dirtyfb is passing ORIGIN_GTT, so it is being ignored by FBC - even
though the hardware tracking is not detecing the WC mmap operations.
So in order to fix that without having to give up the automatic
hardware tracking for GTT mmaps we transform the flush operation from
dirtyfb into a special operation: ORIGIN_DIRTYFB.
This commit fixes all the kms_frontbuffer_tracking subtests that
contain "fbc" and "mmap-wc" in their names and are currently failing
(for a total of 16 subtests).
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cf6761c..78d21c1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -894,6 +894,7 @@ enum fb_op_origin {
ORIGIN_CPU,
ORIGIN_CS,
ORIGIN_FLIP,
+ ORIGIN_DIRTYFB,
};
struct i915_fbc {
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6e3ba5f..cffaaf4a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14479,7 +14479,7 @@ static int intel_user_framebuffer_dirty(struct
drm_framebuffer *fb,
struct drm_i915_gem_object *obj = intel_fb->obj;
mutex_lock(&dev->struct_mutex);
- intel_fb_obj_flush(obj, false, ORIGIN_GTT);
+ intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
mutex_unlock(&dev->struct_mutex);
return 0;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: fix FBC frontbuffer tracking flushing code
From: Paulo Zanoni
Due to the way busy_bits was handled, we were not doing any flushes if
we didn't previously get an invalidate. Since it's possible to get
flushes without an invalidate first, remove the busy_bits early
return.
So now that we don't have the busy_bits guard anymore we'll need the
origin check for the GTT tracking (we were not doing anything on GTT
flushes due to the GTT check at invalidate()).
As a last detail, since we can get multiple consecutive flushes,
disable FBC before updating it, otherwise intel_fbc_update() will just
keep FBC enabled instead of restarting it.
Notice that this does not fix any of the current IGT tests due to the
fact that we still have a few intel_fbc() calls at points where we
also have the frontbuffer tracking calls: we didn't fully convert to
frontbuffer tracking yet. Once we remove those calls and start relying
only on the frontbuffer tracking infrastructure we'll need this patch.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_fbc.c | 13 +++--
drivers/gpu/drm/i915/intel_frontbuffer.c | 2 +-
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f4abce1..2437666 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1240,7 +1240,7 @@ void intel_fbc_invalidate(struct drm_i915_private
*dev_priv,
unsigned int frontbuffer_bits,
enum fb_op_origin origin);
void intel_fbc_flush(struct drm_i915_private *dev_priv,
-unsigned int frontbuffer_bits);
+unsigned int frontbuffer_bits, enum fb_op_origin origin);
const char *intel_no_fbc_reason_str(enum no_fbc_reason reason);
void intel_fbc_cleanup_cfb(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index c271af7..1f97fb5 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -884,22 +884,23 @@ void intel_fbc_invalidate(struct drm_i915_private
*dev_priv,
}
void intel_fbc_flush(struct drm_i915_private *dev_priv,
-unsigned int frontbuffer_bits)
+unsigned int frontbuffer_bits, enum fb_op_origin origin)
{
if (!dev_priv->fbc.enable_fbc)
return;
- mutex_lock(&dev_priv->fbc.lock);
+ if (origin == ORIGIN_GTT)
+ return;
- if (!dev_priv->fbc.busy_bits)
- goto out;
+ mutex_lock(&dev_priv->fbc.lock);
dev_priv->fbc.busy_bits &= ~frontbuffer_bits;
- if (!dev_priv->fbc.busy_bits)
+ if (!dev_priv->fbc.busy_bits) {
+ __intel_fbc_disable(dev_priv);
__intel_fbc_update(dev_priv);
+ }
-out:
mutex_unlock(&dev_priv->fbc.lock);
}
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 777b1d3..ac85357 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -129,7 +129,7 @@ static void intel_frontbuffer_flush(struct drm_device *dev,
intel_edp_drrs_flush(dev, frontbuffer_bits);
intel_psr_flush(dev, frontbuffer_bits, origin);
- intel_fbc_flush(dev_priv, frontbuffer_bits);
+ intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
}
/**
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH igt] kms_frontbuffer_tracking: use the dirty ioctl after MMAP_WC calls
From: Paulo Zanoni
We can't add this to igt_draw since igt_draw doesn't care whether it's
writing on a frontbuffer or not.
PS: the ENOSYS is for Kernels without the patch implementing the
IOCTL.
Signed-off-by: Paulo Zanoni
---
tests/kms_frontbuffer_tracking.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index e162e91..1c45429 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -826,6 +826,21 @@ static struct rect pat4_get_rect(struct fb_region *fb, int
r)
return rect;
}
+static void fb_dirty_ioctl(struct fb_region *fb, struct rect *rect)
+{
+ int rc;
+ drmModeClip clip = {
+ .x1 = rect->x,
+ .x2 = rect->x + rect->w,
+ .y1 = rect->y,
+ .y2 = rect->y + rect->h,
+ };
+
+ rc = drmModeDirtyFB(drm.fd, fb->fb->fb_id, &clip, 1);
+
+ igt_assert(rc == 0 || rc == -ENOSYS);
+}
+
static void draw_rect(struct draw_pattern_info *pattern, struct fb_region *fb,
enum igt_draw_method method, int r)
{
@@ -834,6 +849,9 @@ static void draw_rect(struct draw_pattern_info *pattern,
struct fb_region *fb,
igt_draw_rect_fb(drm.fd, drm.bufmgr, NULL, fb->fb, method,
fb->x + rect.x, fb->y + rect.y,
rect.w, rect.h, rect.color);
+
+ if (method == IGT_DRAW_MMAP_WC)
+ fb_dirty_ioctl(fb, &rect);
}
static void draw_rect_igt_fb(struct draw_pattern_info *pattern,
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
