Re: [PATCH v3 2/6] drm/i915: update cursors asynchronously through atomic
On Wed, Jun 28, 2017 at 05:54:56PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Add support to async updates of cursors by using the new atomic
> interface for that. Basically what this commit does is do what
> intel_legacy_cursor_update() did but through atomic.
>
> v3:
> - set correct vma to new state for cleanup
> - move size checks back to drivers (Ville Syrjälä)
>
> v2:
> - move fb setting to core and use new state (Eric Anholt)
>
> Cc: Daniel Vetter
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 73 +++
> drivers/gpu/drm/i915/intel_display.c | 149
> +-
> 2 files changed, 97 insertions(+), 125 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index a40c82c..1737b8a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -246,11 +246,84 @@ static void intel_plane_atomic_update(struct drm_plane
> *plane,
> }
> }
>
> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> + struct drm_plane_state *state)
> +{
> + struct drm_crtc *crtc = plane->state->crtc;
> + struct drm_crtc_state *crtc_state = crtc->state;
> +
> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
> + return -EINVAL;
> +
> + /*
> + * When crtc is inactive or there is a modeset pending,
> + * wait for it to complete in the slowpath
> + */
> + if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> + return -EINVAL;
> +
> + /*
> + * If any parameters change that may affect watermarks,
> + * take the slowpath. Only changing fb or position should be
> + * in the fastpath.
> + */
> + if (plane->state->crtc != state->crtc ||
> + plane->state->src_w != state->src_w ||
> + plane->state->src_h != state->src_h ||
> + plane->state->crtc_w != state->crtc_w ||
> + plane->state->crtc_h != state->crtc_h ||
> + !plane->state->fb != !state->fb)
> + return -EINVAL;
If we ever expose async updates as an ATOMIC_IOCTL flag then we need to
improve this check a lot, and make it more robust. Otherwise we'll get
things wrong everytime we add a new property (like rotation).
-Daniel
> +
> + return 0;
> +}
> +
> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> + struct drm_plane_state *new_state)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct drm_crtc *crtc = plane->state->crtc;
> + struct drm_framebuffer *old_fb;
> + struct i915_vma *old_vma;
> +
> + old_vma = to_intel_plane_state(plane->state)->vma;
> + old_fb = plane->state->fb;
> +
> + i915_gem_track_fb(intel_fb_obj(old_fb), intel_fb_obj(new_state->fb),
> + intel_plane->frontbuffer_bit);
> +
> + plane->state->src_x = new_state->src_x;
> + plane->state->src_y = new_state->src_y;
> + plane->state->crtc_x = new_state->crtc_x;
> + plane->state->crtc_y = new_state->crtc_y;
> + plane->state->fb = new_state->fb;
> + *to_intel_plane_state(plane->state) = *to_intel_plane_state(new_state);
> +
> + to_intel_plane_state(new_state)->vma = old_vma;
> + new_state->fb = old_fb;
> +
> + if (plane->state->visible) {
> + trace_intel_update_plane(plane, to_intel_crtc(crtc));
> + intel_plane->update_plane(plane,
> + to_intel_crtc_state(crtc->state),
> + to_intel_plane_state(plane->state));
> + } else {
> + trace_intel_disable_plane(plane, to_intel_crtc(crtc));
> + intel_plane->disable_plane(plane, crtc);
> + }
> +
> + mutex_lock(&plane->dev->struct_mutex);
> + intel_cleanup_plane_fb(plane, new_state);
> + mutex_unlock(&plane->dev->struct_mutex);
> +}
> +
> const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
> .prepare_fb = intel_prepare_plane_fb,
> .cleanup_fb = intel_cleanup_plane_fb,
> .atomic_check = intel_plane_atomic_check,
> .atomic_update = intel_plane_atomic_update,
> + .atomic_async_check = intel_plane_atomic_async_check,
> + .atomic_async_update = intel_plane_atomic_async_update,
> };
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 636c64e..736301d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13004,6 +13004,26 @@ static int intel_atomic_commit(struct drm_device
> *dev,
> struct drm_i915_private *dev_priv = to_i915(dev);
> int ret = 0;
>
> + /*
> + * The atomic async update fast path takes care
> + * of avoiding the vblank waits for simple cursor
>
Re: [PATCH v3 1/6] drm/atomic: initial support for asynchronous plane update
On Wed, Jun 28, 2017 at 05:54:55PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> In some cases, like cursor updates, it is interesting to update the
> plane in an asynchronous fashion to avoid big delays. The current queued
> update could be still waiting for a fence to signal and thus block any
> subsequent update until its scan out. In cases like this if we update the
> cursor synchronously through the atomic API it will cause significant
> delays that would even be noticed by the final user.
>
> This patch creates a fast path to jump ahead the current queued state and
> do single planes updates without going through all atomic steps in
> drm_atomic_helper_commit(). We take this path for legacy cursor updates.
>
> For now only single plane updates are supported, but we plan to support
> multiple planes updates and async PageFlips through this interface as well
> in the near future.
>
> v5:
> - improve comments (Eric Anholt)
>
> v4:
> - fix state->crtc NULL check (Archit Taneja)
>
> v3:
> - fix iteration on the wrong crtc state
> - put back code to forbid updates if there is a queued update for
> the same plane (Ville Syrjälä)
> - move size checks back to drivers (Ville Syrjälä)
> - move ASYNC_UPDATE flag addition to its own patch (Ville Syrjälä)
>
> v2:
> - allow updates even if there is a queued update for the same
> plane.
> - fixes on the documentation (Emil Velikov)
> - unconditionally call ->atomic_async_update (Emil Velikov)
> - check for ->atomic_async_update earlier (Daniel Vetter)
> - make ->atomic_async_check() the last step (Daniel Vetter)
> - add ASYNC_UPDATE flag (Eric Anholt)
> - update state in core after ->atomic_async_update (Eric Anholt)
> - update docs (Eric Anholt)
>
> Cc: Daniel Vetter
> Cc: Rob Clark
> Cc: Eric Anholt
> Signed-off-by: Gustavo Padovan
> Reviewed-by: Archit Taneja
> Acked-by: Eric Anholt
Ok I wanted to merge this, but found one more nit. The new
->atomic_async_commit/check hooks are imo driver interface functions (you
call the from the core drm_atomic.c), but you put the hooks into the
helper vtables. Can you pls move them into drm_plane_funcs instead?
The only reason this worked is because I accidentally left a
drm_plane_helper.h include in drm_atomic.c which shouldn't be there ...
Otherwise looks all good, and with that fix has my
Reviewed-by: Daniel Vetter
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 70
>
> drivers/gpu/drm/drm_atomic_helper.c | 35
> include/drm/drm_atomic.h | 2 +
> include/drm/drm_atomic_helper.h | 2 +
> include/drm/drm_modeset_helper_vtables.h | 50 +++
> 5 files changed, 159 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c0f336d..e406688 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -644,6 +644,73 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> return 0;
> }
>
> +static bool drm_atomic_async_check(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *crtc_state;
> + struct drm_crtc_commit *commit;
> + struct drm_plane *__plane, *plane = NULL;
> + struct drm_plane_state *__plane_state, *plane_state = NULL;
> + const struct drm_plane_helper_funcs *funcs;
> + int i, j, n_planes = 0;
> +
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + if (drm_atomic_crtc_needs_modeset(crtc_state))
> + return false;
> + }
> +
> + for_each_new_plane_in_state(state, __plane, __plane_state, i) {
> + n_planes++;
> + plane = __plane;
> + plane_state = __plane_state;
> + }
> +
> + /* FIXME: we support only single plane updates for now */
> + if (!plane || n_planes != 1)
> + return false;
> +
> + if (!plane_state->crtc)
> + return false;
> +
> + funcs = plane->helper_private;
> + if (!funcs->atomic_async_update)
> + return false;
> +
> + if (plane_state->fence)
> + return false;
> +
> + /*
> + * Don't do an async update if there is an outstanding commit modifying
> + * the plane. This prevents our async update's changes from getting
> + * overridden by a previous synchronous update's state.
> + */
> + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> + if (plane->crtc != crtc)
> + continue;
> +
> + spin_lock(&crtc->commit_lock);
> + commit = list_first_entry_or_null(&crtc->commit_list,
> + struct drm_crtc_commit,
> + commit_entry);
> + if (!commit) {
> +
[PATCH] drm/atomic: Drop helper include from drm_atomic.c
Core code should never have to look at helper stuff, to make sure that all helper code is 100% optional and can be overriden. Cc: Gustavo Padovan Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_atomic.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 095e87278a88..09ca662fcd35 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include -- 2.11.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 196197] [drm:r600_ring_test [radeon]] *ERROR* radeon: ring 0 test failed (scratch(0x8504)=0xCAFEDEAD)
https://bugzilla.kernel.org/show_bug.cgi?id=196197 --- Comment #6 from Michel Dänzer (mic...@daenzer.net) --- That's not the end of the bisection yet. Every time you run "git bisect good/bad/skip", it will check out a new commit that you need to compile and test, and print the estimated number of tests remaining. After the latter reaches 0, it should either print details of the first bad commit, or the minimal list of candidates. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101499] Black screen when detaching HDMI cable (AMD A10-9620P)
https://bugs.freedesktop.org/show_bug.cgi?id=101499 Michel Dänzer changed: What|Removed |Added CC||harry.wentl...@amd.com --- Comment #21 from Michel Dänzer --- Sounds like either DC is still missing something for scanning out from GTT, or there may indeed be additional constraints on the system memory used for it. Harry, have you guys tested GTT scanout? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/6] drm/atomic: initial support for asynchronous plane update
On Fri, Jun 30, 2017 at 09:30:50AM +0200, Daniel Vetter wrote:
> On Wed, Jun 28, 2017 at 05:54:55PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan
> >
> > In some cases, like cursor updates, it is interesting to update the
> > plane in an asynchronous fashion to avoid big delays. The current queued
> > update could be still waiting for a fence to signal and thus block any
> > subsequent update until its scan out. In cases like this if we update the
> > cursor synchronously through the atomic API it will cause significant
> > delays that would even be noticed by the final user.
> >
> > This patch creates a fast path to jump ahead the current queued state and
> > do single planes updates without going through all atomic steps in
> > drm_atomic_helper_commit(). We take this path for legacy cursor updates.
> >
> > For now only single plane updates are supported, but we plan to support
> > multiple planes updates and async PageFlips through this interface as well
> > in the near future.
> >
> > v5:
> > - improve comments (Eric Anholt)
> >
> > v4:
> > - fix state->crtc NULL check (Archit Taneja)
> >
> > v3:
> > - fix iteration on the wrong crtc state
> > - put back code to forbid updates if there is a queued update for
> > the same plane (Ville Syrjälä)
> > - move size checks back to drivers (Ville Syrjälä)
> > - move ASYNC_UPDATE flag addition to its own patch (Ville Syrjälä)
> >
> > v2:
> > - allow updates even if there is a queued update for the same
> > plane.
> > - fixes on the documentation (Emil Velikov)
> > - unconditionally call ->atomic_async_update (Emil Velikov)
> > - check for ->atomic_async_update earlier (Daniel Vetter)
> > - make ->atomic_async_check() the last step (Daniel Vetter)
> > - add ASYNC_UPDATE flag (Eric Anholt)
> > - update state in core after ->atomic_async_update (Eric Anholt)
> > - update docs (Eric Anholt)
> >
> > Cc: Daniel Vetter
> > Cc: Rob Clark
> > Cc: Eric Anholt
> > Signed-off-by: Gustavo Padovan
> > Reviewed-by: Archit Taneja
> > Acked-by: Eric Anholt
>
> Ok I wanted to merge this, but found one more nit. The new
> ->atomic_async_commit/check hooks are imo driver interface functions (you
> call the from the core drm_atomic.c), but you put the hooks into the
> helper vtables. Can you pls move them into drm_plane_funcs instead?
>
> The only reason this worked is because I accidentally left a
> drm_plane_helper.h include in drm_atomic.c which shouldn't be there ...
Otoh this does all feel supremely like helper code, so maybe the better
option would be to move all the code you're adding here into
drm_atomic_helper.c, (with function prefixes changed). You already call
async_commit from the helper's atomic_commit, and doing the same for the
check side would make things more symmetric.
I think that way round would be even cleaner separation between core code
and helpers.
-Daniel
>
> Otherwise looks all good, and with that fix has my
>
> Reviewed-by: Daniel Vetter
>
> Thanks, Daniel
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 70
> >
> > drivers/gpu/drm/drm_atomic_helper.c | 35
> > include/drm/drm_atomic.h | 2 +
> > include/drm/drm_atomic_helper.h | 2 +
> > include/drm/drm_modeset_helper_vtables.h | 50 +++
> > 5 files changed, 159 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index c0f336d..e406688 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -644,6 +644,73 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc,
> > return 0;
> > }
> >
> > +static bool drm_atomic_async_check(struct drm_atomic_state *state)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_crtc_commit *commit;
> > + struct drm_plane *__plane, *plane = NULL;
> > + struct drm_plane_state *__plane_state, *plane_state = NULL;
> > + const struct drm_plane_helper_funcs *funcs;
> > + int i, j, n_planes = 0;
> > +
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > + if (drm_atomic_crtc_needs_modeset(crtc_state))
> > + return false;
> > + }
> > +
> > + for_each_new_plane_in_state(state, __plane, __plane_state, i) {
> > + n_planes++;
> > + plane = __plane;
> > + plane_state = __plane_state;
> > + }
> > +
> > + /* FIXME: we support only single plane updates for now */
> > + if (!plane || n_planes != 1)
> > + return false;
> > +
> > + if (!plane_state->crtc)
> > + return false;
> > +
> > + funcs = plane->helper_private;
> > + if (!funcs->atomic_async_update)
> > + return false;
> > +
> > + if (plane_state->fence)
> > + return false;
> > +
> > + /*
> > +* Don't do an async update if the
[PATCH] drm/msm: fix an integer overflow test
We recently added an integer overflow check but it needs an additional
tweak to work properly on 32 bit systems.
The problem is that we're doing the right hand side of the assignment as
type unsigned long so the max it will have an integer overflow instead
of being larger than SIZE_MAX. That means the "sz > SIZE_MAX" condition
is never true even on 32 bit systems. We need to first cast it to u64
and then do the math.
Fixes: 4a630fadbb29 ("drm/msm: Fix potential buffer overflow issue")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index 6bfca7470141..8095658e8cb4 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -34,8 +34,8 @@ static struct msm_gem_submit *submit_create(struct drm_device
*dev,
struct msm_gpu *gpu, uint32_t nr_bos, uint32_t nr_cmds)
{
struct msm_gem_submit *submit;
- uint64_t sz = sizeof(*submit) + (nr_bos * sizeof(submit->bos[0])) +
- (nr_cmds * sizeof(submit->cmd[0]));
+ uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) +
+ ((u64)nr_cmds * sizeof(submit->cmd[0]));
if (sz > SIZE_MAX)
return NULL;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: rcar-du: remove an unneeded NULL check
"params" can't be NULL here. The next lines assume that we either
hit the break statement of "params->mpixelclock == ~0UL". The
inconsistent NULL checking makes static checkers complain. I've just
removed the test.
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
index 7539626b8ebd..dc85b53d58ef 100644
--- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
@@ -45,7 +45,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi,
{
const struct rcar_hdmi_phy_params *params = rcar_hdmi_phy_params;
- for (; params && params->mpixelclock != ~0UL; ++params) {
+ for (; params->mpixelclock != ~0UL; ++params) {
if (mpixelclock <= params->mpixelclock)
break;
}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/8] drm: Allow DSI devices to be registered before the host registers.
On 29.06.2017 17:22, Eric Anholt wrote: > Andrzej Hajda writes: > >> On 29.06.2017 07:03, Archit Taneja wrote: >>> On 06/28/2017 01:28 AM, Eric Anholt wrote: When a mipi_dsi_host is registered, the DT is walked to find any child nodes with compatible strings. Those get registered as DSI devices, and most DSI panel drivers are mipi_dsi_drivers that attach to those nodes. There is one special case currently, the adv7533 bridge, where the bridge probes on I2C, and during the bridge attach step it looks up the mipi_dsi_host and registers the mipi_dsi_device (for its own stub mipi_dsi_driver). For the Raspberry Pi panel, though, we also need to attach on I2C (our control bus), but don't have a bridge driver. The lack of a bridge's attach() step like adv7533 uses means that we aren't able to delay the mipi_dsi_device creation until the mipi_dsi_host is present. To fix this, we extend mipi_dsi_device_register_full() to allow being called with a NULL host, which puts the device on a queue waiting for a host to appear. When a new host is registered, we fill in the host value and finish the device creation process. >>> This is quite a nice idea. The only bothering thing is the info.of_node >>> usage >>> varies between child nodes (mipi_dsi_devs) and non-child nodes (i2c control >>> bus). >>> >>> For DSI children expressed in DT, the of_node in info holds the DT node >>> corresponding to the DSI child itself. For non-DT ones, this patch assumes >>> that info.of_node stores the DSI host DT node. I think it should be okay as >>> long as we mention the usage in a comment somewhere. The other option is to >>> have a new info.host_node field to keep a track of the host DT node. >> Field abuse is not a biggest issue. >> >> This patch changes totally semantic of mipi_dsi_device_register_full. >> Currently semantic of *_device_register* function is to create and add >> device to existing bus, ie after return we have device attached to bus, >> so it can be instantly used. With this change function can return some >> unattached device, without warranty it will be ever attached - kind of >> hidden deferring. Such change looks for me quite dangerous, even if it >> looks convenient in this case. > It only changes the semantic if you past in a NULL host, from "oops" to > "do something useful". > >> As discussed in other thread more appealing solution for me would be: >> 1. host creates dsi bus, but doesn't call component_add as it does not >> have all required resources. >> 2. host waits for all required dsi devs attached, gets registered panels >> or bridges and calls component_add after that. >> 3. in bind phase it has all it needs, hasn't it? >> >> I did not spent much time on this idea, so I cannot guarantee it has not >> fundamental issues :) > If component_add() isn't called during probe, then DSI would just get > skipped during bind as far as I know. No, drm master will wait till all components are present. > > I *think* what you're thinking is moving DSI host register to probe, yes, this way you will allow to create dsi devices. > and > then panel lookup stays in bind. no, panel lookup will be performed in dsi_host attach callback, and component_add will be called also there, if all required panels/bridges are get. > That seems much more risky to me -- do > we know for sure that no mipi_dsi_device will do any DSI transactions > during its probe? I would expect some of them to. I think it is irrelevant, with the current design only transactions between prepare/unprepare callbacks have chances to succeed. Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, REGRESSION] v4.12-rc: omapdrm fails to probe on Nokia N900
On 30/06/17 09:41, Tomi Valkeinen wrote:
> So, I don't know... I guess I need to try to invent some horrible hacks
> around the driver to somehow manage the omap3 problems. Perhaps
> disabling/enabling the outputs when sync lost happens...
Well, I tried that (attached), but it didn't work either. For some
reason the error worker seems to stop after the disable. Possibly the
irq flood keeps it from running, so maybe it should catch all the errors
(I see underflows too).
Sorry, but I can't use more time on this today, and I'm leaving for
vacation today. I hope Laurent can help during my absence.
We could try reverting the patch you mention, but I think it doesn't
cause the problem.
Did you have CONFIG_DRM_OMAP_CONNECTOR_ANALOG_TV enabled earlier when
things worked? If you didn't, and the dts did not contain display
aliases, I think the omapdrm may have started without TV. So maybe the
TV side is the culprit, somehow (I couldn't find anything when I looked
at that side either).
Tomi
From c4ceb8934dbfa51bc966b927b17394c4b622712c Mon Sep 17 00:00:00 2001
From: Tomi Valkeinen
Date: Fri, 30 Jun 2017 11:39:53 +0300
Subject: [PATCH] drm/omap: hack error worker
---
drivers/gpu/drm/omapdrm/omap_crtc.c | 69 -
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index dccd03726796..eb36b35f5eb8 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -36,12 +36,15 @@ struct omap_crtc {
struct videomode vm;
- bool ignore_digit_sync_lost;
+ bool ignore_sync_lost;
bool enabled;
bool pending;
wait_queue_head_t pending_wait;
struct drm_pending_vblank_event *event;
+
+ struct work_struct error_work;
+ u32 error_channels;
};
/* -
@@ -157,7 +160,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
* Digit output produces some sync lost interrupts during the
* first frame when enabling, so we need to ignore those.
*/
- omap_crtc->ignore_digit_sync_lost = true;
+ omap_crtc->ignore_sync_lost = true;
}
framedone_irq = priv->dispc_ops->mgr_get_framedone_irq(channel);
@@ -191,7 +194,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
}
if (omap_crtc->channel == OMAP_DSS_CHANNEL_DIGIT) {
- omap_crtc->ignore_digit_sync_lost = false;
+ omap_crtc->ignore_sync_lost = false;
/* make sure the irq handler sees the value above */
mb();
}
@@ -263,17 +266,65 @@ static const struct dss_mgr_ops mgr_ops = {
* Setup, Flush and Page Flip
*/
+static void omap_crtc_error_worker(struct work_struct *work)
+{
+ struct omap_crtc *omap_crtc = container_of(work, struct omap_crtc, error_work);
+ struct drm_crtc *crtc = &omap_crtc->base;
+ struct drm_device *dev = omap_crtc->base.dev;
+ struct omap_drm_private *priv = dev->dev_private;
+
+ drm_modeset_lock(&crtc->mutex, NULL);
+
+ dev_warn(dev->dev, "sync lost on %s, enabling & disabling...\n",
+ omap_crtc->name);
+
+ priv->dispc_ops->mgr_enable(omap_crtc->channel, false);
+
+ msleep(50);
+ dev_warn(dev->dev, "sync lost enabling %s\n",
+ omap_crtc->name);
+
+ priv->dispc_ops->mgr_enable(omap_crtc->channel, true);
+
+ msleep(50);
+
+ dev_warn(dev->dev, "sync lost recovery done on on %s\n",
+ omap_crtc->name);
+
+ omap_crtc->ignore_sync_lost = false;
+ /* make sure the irq handler sees the value above */
+ mb();
+
+ drm_modeset_unlock(&crtc->mutex);
+}
+
void omap_crtc_error_irq(struct drm_crtc *crtc, uint32_t irqstatus)
{
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+ struct drm_device *dev = omap_crtc->base.dev;
+ struct omap_drm_private *priv = dev->dev_private;
+ enum omap_channel channel = omap_crtc_channel(crtc);
+ u32 sync_lost_irq;
+ bool sync_lost;
+
+ sync_lost_irq = priv->dispc_ops->mgr_get_sync_lost_irq(channel);
- if (omap_crtc->ignore_digit_sync_lost) {
- irqstatus &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
- if (!irqstatus)
- return;
+ sync_lost = irqstatus & sync_lost_irq;
+
+ if (sync_lost) {
+ if (omap_crtc->ignore_sync_lost) {
+ irqstatus &= ~sync_lost_irq;
+ } else {
+ /* error worker will set this to false */
+ omap_crtc->ignore_sync_lost = true;
+ schedule_work(&omap_crtc->error_work);
+ }
}
- DRM_ERROR_RATELIMITED("%s: errors: %08x\n", omap_crtc->name, irqstatus);
+ if (!irqstatus)
+ return;
+
+ printk("%s: errors: %08x\n", omap_crtc->name, irqstatus);
}
void omap_crtc_vblank_irq(struct drm_crtc *crtc)
@@ -612,6 +663,8 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
init_waitqueue_head(&omap_crtc->pending_wait);
+ INIT_WORK(&omap_crtc->error_work, omap_crtc_error_worker);
+
omap_crtc->channel = channel;
omap_crtc->name = channel_names[channel];
--
2.7.4
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists
Re: [BISECTED, REGRESSION] v4.12-rc: omapdrm fails to probe on Nokia N900
On 30/06/17 11:58, Aaro Koskinen wrote:
> Hi,
>
> On Fri, Jun 30, 2017 at 09:41:35AM +0300, Tomi Valkeinen wrote:
>> On 29/06/17 21:50, Aaro Koskinen wrote:
>>> On Thu, Jun 15, 2017 at 09:51:06AM +0300, Tomi Valkeinen wrote:
On 15/06/17 01:11, Aaro Koskinen wrote:
> When booting v4.12-rc5 on Nokia N900, omapdrm fails to probe and there
> is no display.
Are you sure it doesn't probe? It fails the omapdss_stack_is_ready()
check?
>>>
>>> It appears the reason was that I didn't have
>>> CONFIG_DRM_OMAP_CONNECTOR_ANALOG_TV enabled.
>>>
>>> I think that's wrong. I don't own an analog TV, so why should I enable
>>> such option to get device's built-in display working?
>>
>> Indeed. Unfortunately I don't have a solution for that.
>>
>> DRM doesn't support adding devices after probe. So at omapdrm probe time
>> we have to decide which displays to use. In the dts file, n900 defines
>> the lcd and analog tv. omapdrm sees those, and, of course, must wait
>> until their respective drivers have probed. If you don't have the
>> display driver enabled, it's never loaded and omapdrm never probes as it
>> keeps waiting for those.
>
> Could you at least print some kind of message early in the boot ("omapdrm
> is waiting for drivers for display x and y")?
That could be quite spammy. omapdrm will defer probe if the displays are
not present, and the deferred probing machinery will then cause a new
omapdrm probe later. That can happen a lot of times before the drivers
are there.
Tomi
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rcar-du: remove an unneeded NULL check
Hi Dan,
Thank you for the patch.
On Friday 30 Jun 2017 11:00:12 Dan Carpenter wrote:
> "params" can't be NULL here. The next lines assume that we either
> hit the break statement of "params->mpixelclock == ~0UL". The
> inconsistent NULL checking makes static checkers complain. I've just
> removed the test.
>
> Signed-off-by: Dan Carpenter
Reviewed-by: Laurent Pinchart
and taken in my tree for v4.14.
> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c index 7539626b8ebd..dc85b53d58ef
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> @@ -45,7 +45,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi,
> {
> const struct rcar_hdmi_phy_params *params = rcar_hdmi_phy_params;
>
> - for (; params && params->mpixelclock != ~0UL; ++params) {
> + for (; params->mpixelclock != ~0UL; ++params) {
> if (mpixelclock <= params->mpixelclock)
> break;
> }
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5] drm/sun4i: hdmi: Implement I2C adapter for A10s DDC bus
Hi, On Fri, Jun 30, 2017 at 08:22:13AM +1000, Jonathan Liu wrote: > Hi Maxime, > > On 30 June 2017 at 01:56, Maxime Ripard > wrote: > > On Wed, Jun 28, 2017 at 08:39:33PM +1000, Jonathan Liu wrote: > >> >> + u32 int_status; > >> >> + u32 fifo_status; > >> >> + /* Read needs empty flag unset, write needs full flag unset */ > >> >> + u32 flag = read ? SUN4I_HDMI_DDC_FIFO_STATUS_EMPTY : > >> >> + SUN4I_HDMI_DDC_FIFO_STATUS_FULL; > >> >> + int ret; > >> >> + > >> >> + /* Wait until error or FIFO ready */ > >> >> + ret = readl_poll_timeout(hdmi->base + > >> >> SUN4I_HDMI_DDC_INT_STATUS_REG, > >> >> + int_status, > >> >> + is_err_status(int_status) || > >> >> + is_fifo_flag_unset(hdmi, &fifo_status, > >> >> flag), > >> >> + min(len, SUN4I_HDMI_DDC_FIFO_SIZE) * > >> >> byte_time, > >> >> + 10); > >> >> + > >> >> + if (is_err_status(int_status)) > >> >> + return -EIO; > >> >> + if (ret) > >> >> + return -ETIMEDOUT; > >> > > >> > Why not just have > >> > ret = readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_FIFO_STATUS_REG, > >> > reg, > >> > !(reg & flag), 100, 10); > >> > > >> > if (ret < 0) > >> > if (is_err_status()) > >> > return -EIO; > >> > return ret; > >> > > >> > > >> > >> If I check error status after readl_poll_timeout and there is an error > >> (e.g. the I2C address does not have a corresponding device connected > >> or nothing connected to HDMI port) it will keep checking the fifo > >> status even though error bit is set in the int status and then timeout > >> after 100 ms. If it checks the int status register at the same time, > >> it will error after 100 nanoseconds. I don't want to introduce > >> unnecessary delays considering part of the reason for adding this > >> driver to make it more usable for non-standard use cases. > > > > Well, polling for 100ms doesn't seem great either. What was the > > rationale behind that timeout? > > > > When an error occurs one of the error bits will be set in the > INT_STATUS register so this is detected very quickly if I check the > INT_STATUS and FIFO_STATUS at the same time. The 100 ms timeout is in > case the I2C slave does clock stretching in which case the transfer > may take longer than the predicted time. 100ms isn't stretching anymore, it's worse than that. > > And we can also reverse the check and look at the INT_STATUS > > register. The errors will be there, and we can program the threshold > > we want in both directions and use the > > DDC_FIFO_Request_Interrupt_Status bit. > > > > I did try that when I was doing the v3 patch but I couldn't get it to > work as mentioned previously in the v3 patch discussion. I programmed > the FIFO_RX_TRIGGER_THRES and FIFO_TX_TRIGGER_THRES in DDC_FIFO_Ctrl > register at the same time as setting FIFO_Address_Clear but the > request interrupt status bit did not get updated to the appropriate > state that is consistent with the FIFO level and the thresholds. I did > try this several times for subsequent patch versions without success. What values did you set it to? Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm: Add old state pointer to CRTC .enable() helper function
The old state is useful for drivers that need to perform operations at
enable time that depend on the transition between the old and new
states.
While at it, rename the operation to .atomic_enable() to be consistent
with .atomic_disable(), as the .enable() operation is used by atomic
helpers only.
Signed-off-by: Laurent Pinchart
Acked-by: Maxime Ripard # for sun4i
Acked-by: Philipp Zabel # for imx-drm and mediatek
Acked-by: Alexey Brodkin # for arcpgu
Acked-by: Boris Brezillon # for atmel-hlcdc
Acked-by: Liviu Dudau # for hdlcd and mali-dp
Acked-by: Stefan Agner # for fsl-dcu
Tested-by: Philippe Cornu # for stm
Acked-by: Philippe Cornu # for stm
Acked-by: Vincent Abriou # for sti
Reviewed-by: Thomas Hellstrom # for vmwgfx
---
Changes since v1:
- Update kerneldoc
---
drivers/gpu/drm/arc/arcpgu_crtc.c | 5 ++-
drivers/gpu/drm/arm/hdlcd_crtc.c| 5 ++-
drivers/gpu/drm/arm/malidp_crtc.c | 5 ++-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 5 ++-
drivers/gpu/drm/drm_atomic_helper.c | 7 ++--
drivers/gpu/drm/drm_simple_kms_helper.c | 5 ++-
drivers/gpu/drm/exynos/exynos_drm_crtc.c| 5 ++-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 5 ++-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 5 ++-
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 ++-
drivers/gpu/drm/imx/ipuv3-crtc.c| 5 ++-
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 ++-
drivers/gpu/drm/meson/meson_crtc.c | 5 ++-
drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c| 5 ++-
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 5 ++-
drivers/gpu/drm/omapdrm/omap_crtc.c | 5 ++-
drivers/gpu/drm/qxl/qxl_display.c | 5 ++-
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 5 ++-
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 5 ++-
drivers/gpu/drm/sti/sti_crtc.c | 5 ++-
drivers/gpu/drm/stm/ltdc.c | 5 ++-
drivers/gpu/drm/sun4i/sun4i_crtc.c | 5 ++-
drivers/gpu/drm/tegra/dc.c | 5 ++-
drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 8 +++-
drivers/gpu/drm/vc4/vc4_crtc.c | 5 ++-
drivers/gpu/drm/virtio/virtgpu_display.c| 5 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 7 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 7 ++--
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 5 ++-
drivers/gpu/drm/zte/zx_vou.c| 5 ++-
include/drm/drm_modeset_helper_vtables.h| 56 ++---
31 files changed, 128 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c
b/drivers/gpu/drm/arc/arcpgu_crtc.c
index 1f306781c9d5..c9bc6a90ac83 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -119,7 +119,8 @@ static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc
*crtc)
clk_set_rate(arcpgu->clk, m->crtc_clock * 1000);
}
-static void arc_pgu_crtc_enable(struct drm_crtc *crtc)
+static void arc_pgu_crtc_atomic_enable(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_state)
{
struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
@@ -161,9 +162,9 @@ static const struct drm_crtc_helper_funcs
arc_pgu_crtc_helper_funcs = {
.mode_set = drm_helper_crtc_mode_set,
.mode_set_base = drm_helper_crtc_mode_set_base,
.mode_set_nofb = arc_pgu_crtc_mode_set_nofb,
- .enable = arc_pgu_crtc_enable,
.disable= arc_pgu_crtc_disable,
.atomic_begin = arc_pgu_crtc_atomic_begin,
+ .atomic_enable = arc_pgu_crtc_atomic_enable,
};
static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index d67b6f15e8b8..2b7f4f05d91f 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -165,7 +165,8 @@ static void hdlcd_crtc_mode_set_nofb(struct drm_crtc *crtc)
clk_set_rate(hdlcd->clk, m->crtc_clock * 1000);
}
-static void hdlcd_crtc_enable(struct drm_crtc *crtc)
+static void hdlcd_crtc_atomic_enable(struct drm_crtc *crtc,
+struct drm_crtc_state *old_state)
{
struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
@@ -218,10 +219,10 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
- .enable = hdlcd_crtc_enable,
.disable= hdlcd_crtc_disable,
.atomic_check = hdlcd_crtc_atomic_check,
.atomic_begin = hdlcd_crtc_atomic_begin,
+ .atomic_enable = hdlcd_crtc_atomic_enable,
};
static int hdlcd_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 4bb38a21efec..8e5b1c018
[PATCH v2 3/3] drm: vmwgfx: Remove the legacy CRTC .prepare() helper operations
The CRTC .prepare() helper operation is legacy code, drivers should
use the .atomic_disable() operation instead.
When a CRTC is temporarily disabled prior to a mode set, the atomic
helpers call the .prepare() operation if provided instead of the
.atomic_disable() operation. In order to avoid modifying the driver's
behaviour that has an empty .prepare() implementation, we need to return
from the .atomic_disable() operation without performing any action if
the CRTC will be reenabled.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 15 +++
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 +++---
2 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 854403509216..bdf6349de250 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -259,17 +259,6 @@ static void vmw_sou_crtc_mode_set_nofb(struct drm_crtc
*crtc)
}
/**
- * vmw_sou_crtc_helper_prepare - Noop
- *
- * @crtc: CRTC associated with the new screen
- *
- * Prepares the CRTC for a mode set, but we don't need to do anything here.
- */
-static void vmw_sou_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-/**
* vmw_sou_crtc_atomic_enable - Noop
*
* @crtc: CRTC associated with the new screen
@@ -299,6 +288,9 @@ static void vmw_sou_crtc_atomic_disable(struct drm_crtc
*crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
sou = vmw_crtc_to_sou(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -574,7 +566,6 @@ drm_plane_helper_funcs vmw_sou_primary_plane_helper_funcs =
{
};
static const struct drm_crtc_helper_funcs vmw_sou_crtc_helper_funcs = {
- .prepare = vmw_sou_crtc_helper_prepare,
.mode_set_nofb = vmw_sou_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ed9404a7f457..c3bd4a012b64 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -406,12 +406,6 @@ static void vmw_stdu_crtc_mode_set_nofb(struct drm_crtc
*crtc)
crtc->x, crtc->y);
}
-
-static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-
static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
{
@@ -446,6 +440,9 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc
*crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
stdu = vmw_crtc_to_stdu(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -1416,7 +1413,6 @@ drm_plane_helper_funcs
vmw_stdu_primary_plane_helper_funcs = {
};
static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
- .prepare = vmw_stdu_crtc_helper_prepare,
.mode_set_nofb = vmw_stdu_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] Cleanup CRTC .enable()/.disable() cargo-cult
Hello, The first version of this patch series removed legacy CRTC helper operations from most atomic drivers, and got partially merged in the drm-misc tree. This second version is a repost of the two unmerged patches from the original series, with an additional patch to complete removal of the last atomic user of the legacy CRTC .prepare() helper operation. The patches are based on top of the drm-misc-next branch and have been compile-tested only except for rcar-du-drm that has been tested on real hardware. Given the high risk of conflicts I would like to get them merged as soon as possible (after, of course, proper review and testing). Laurent Pinchart (3): drm: Add old state pointer to CRTC .enable() helper function drm: Convert atomic drivers from CRTC .disable() to .atomic_disable() drm: vmwgfx: Remove the legacy CRTC .prepare() helper operations drivers/gpu/drm/arc/arcpgu_crtc.c | 10 +++-- drivers/gpu/drm/arm/hdlcd_crtc.c| 10 +++-- drivers/gpu/drm/arm/malidp_crtc.c | 10 +++-- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 10 +++-- drivers/gpu/drm/drm_atomic_helper.c | 7 ++-- drivers/gpu/drm/drm_simple_kms_helper.c | 10 +++-- drivers/gpu/drm/exynos/exynos_drm_crtc.c| 10 +++-- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 5 ++- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 10 +++-- drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 10 +++-- drivers/gpu/drm/imx/ipuv3-crtc.c| 5 ++- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 10 +++-- drivers/gpu/drm/meson/meson_crtc.c | 10 +++-- drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c| 10 +++-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 10 +++-- drivers/gpu/drm/omapdrm/omap_crtc.c | 10 +++-- drivers/gpu/drm/qxl/qxl_display.c | 10 +++-- drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 10 +++-- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +++-- drivers/gpu/drm/sti/sti_crtc.c | 10 +++-- drivers/gpu/drm/stm/ltdc.c | 10 +++-- drivers/gpu/drm/sun4i/sun4i_crtc.c | 10 +++-- drivers/gpu/drm/tegra/dc.c | 10 +++-- drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 16 ++- drivers/gpu/drm/vc4/vc4_crtc.c | 10 +++-- drivers/gpu/drm/virtio/virtgpu_display.c| 10 +++-- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 14 --- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 29 + drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 20 - drivers/gpu/drm/zte/zx_vou.c| 10 +++-- include/drm/drm_modeset_helper_vtables.h| 56 ++--- 31 files changed, 221 insertions(+), 161 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm: Convert atomic drivers from CRTC .disable() to .atomic_disable()
The CRTC .disable() helper operation is deprecated for atomic drivers,
the new .atomic_disable() helper operation being preferred. Convert all
atomic drivers to .atomic_disable() to avoid cargo-cult use of
.disable() in new drivers.
Signed-off-by: Laurent Pinchart
Acked-by: Maxime Ripard # for sun4i
Acked-by: Philipp Zabel # for mediatek
Acked-by: Alexey Brodkin # for arcpgu
Acked-by: Boris Brezillon # for atmel-hlcdc
Tested-by: Philippe Cornu # for stm
Acked-by: Philippe Cornu # for stm
Acked-by: Vincent Abriou # for sti
Reviewed-by: Thomas Hellstrom # for vmwgfx
---
drivers/gpu/drm/arc/arcpgu_crtc.c | 5 +++--
drivers/gpu/drm/arm/hdlcd_crtc.c| 5 +++--
drivers/gpu/drm/arm/malidp_crtc.c | 5 +++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 5 +++--
drivers/gpu/drm/drm_simple_kms_helper.c | 5 +++--
drivers/gpu/drm/exynos/exynos_drm_crtc.c| 5 +++--
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 5 +++--
drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 +++--
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 +++--
drivers/gpu/drm/meson/meson_crtc.c | 5 +++--
drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c| 5 +++--
drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 5 +++--
drivers/gpu/drm/omapdrm/omap_crtc.c | 5 +++--
drivers/gpu/drm/qxl/qxl_display.c | 5 +++--
drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 5 +++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 5 +++--
drivers/gpu/drm/sti/sti_crtc.c | 5 +++--
drivers/gpu/drm/stm/ltdc.c | 5 +++--
drivers/gpu/drm/sun4i/sun4i_crtc.c | 5 +++--
drivers/gpu/drm/tegra/dc.c | 5 +++--
drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 8 +++-
drivers/gpu/drm/vc4/vc4_crtc.c | 5 +++--
drivers/gpu/drm/virtio/virtgpu_display.c| 5 +++--
drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 7 ---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 7 ---
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 5 +++--
drivers/gpu/drm/zte/zx_vou.c| 5 +++--
27 files changed, 87 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c
b/drivers/gpu/drm/arc/arcpgu_crtc.c
index c9bc6a90ac83..1859dd3ad622 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -130,7 +130,8 @@ static void arc_pgu_crtc_atomic_enable(struct drm_crtc
*crtc,
ARCPGU_CTRL_ENABLE_MASK);
}
-static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
+static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_state)
{
struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
@@ -162,9 +163,9 @@ static const struct drm_crtc_helper_funcs
arc_pgu_crtc_helper_funcs = {
.mode_set = drm_helper_crtc_mode_set,
.mode_set_base = drm_helper_crtc_mode_set_base,
.mode_set_nofb = arc_pgu_crtc_mode_set_nofb,
- .disable= arc_pgu_crtc_disable,
.atomic_begin = arc_pgu_crtc_atomic_begin,
.atomic_enable = arc_pgu_crtc_atomic_enable,
+ .atomic_disable = arc_pgu_crtc_atomic_disable,
};
static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 2b7f4f05d91f..16e1e20cf04c 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -176,7 +176,8 @@ static void hdlcd_crtc_atomic_enable(struct drm_crtc *crtc,
drm_crtc_vblank_on(crtc);
}
-static void hdlcd_crtc_disable(struct drm_crtc *crtc)
+static void hdlcd_crtc_atomic_disable(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_state)
{
struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
@@ -219,10 +220,10 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
}
static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
- .disable= hdlcd_crtc_disable,
.atomic_check = hdlcd_crtc_atomic_check,
.atomic_begin = hdlcd_crtc_atomic_begin,
.atomic_enable = hdlcd_crtc_atomic_enable,
+ .atomic_disable = hdlcd_crtc_atomic_disable,
};
static int hdlcd_plane_atomic_check(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
b/drivers/gpu/drm/arm/malidp_crtc.c
index 8e5b1c0181ab..3615d18a7ddf 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -70,7 +70,8 @@ static void malidp_crtc_atomic_enable(struct drm_crtc *crtc,
drm_crtc_vblank_on(crtc);
}
-static void malidp_crtc_disable(struct drm_crtc *crtc)
+static void malidp_crtc_atomic_disable(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_state)
{
struct malidp_drm *malidp =
[Bug 101499] Black screen when detaching HDMI cable (AMD A10-9620P)
https://bugs.freedesktop.org/show_bug.cgi?id=101499 --- Comment #22 from Carlo Caione --- Just to keep you updated we have verified that with a bigger VRAM this is not reproducible anymore. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/8] ASoC: AMD: added condition checks for CZ specific code
On Thu, Jun 29, 2017 at 12:58:02PM +, Mukunda, Vijendar wrote: > -Original Message- > From: Mark Brown [mailto:broo...@kernel.org] > Sent: Wednesday, June 28, 2017 11:36 PM > To: Alex Deucher Please fix your mail client to quote mails in a more normal fashion, this looks pretty broken... > >These defines are being added in the middle of a file but CHIP_STONEY is > >also used in another file in the previous patch (and apparently extensively > >throughout the DRM driver already). This is obviously not good, >we > >shouldn't have multiple copies of the definition. ...especially in that it's reflowing the message it's replying to to cause 80 column problems and has serious problems in that regard itself. > We will modify code to use single definition for CHIP_STONEY and CHIP_CARRIZO. > There are only two chip sets based on ACP 2.x design(Carrizo and Stoney). > Our future Chip sets going to use different design based on next ACP IP > version. Write the code well, that way we don't have bad patterns in the codebase and if plans change with regard to new variants you're covered. > In the current patch, Condition checks added for Carrizo for setting SRAM > BANK state. > Memory Gating is disabled in Stoney,i.e SRAM Bank's won't be turned off. The > default state for SRAM banks is ON. > As Memory Gating is disabled, there is no need to add condition checks for > Stoney to set SRAM Bank state. Some documentation of this in the code would be good. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 13/15] drm/i915: prepare csc unit for YCBCR HDMI output
On Fri, 2017-06-30 at 11:33 +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 6/29/2017 5:38 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > To support ycbcr HDMI output, we need a pipe CSC block to
> > > do the RGB->YCBCR conversion, as the blender output is in RGB.
> > >
> > > Current Intel platforms have only one pipe CSC unit, so
> > > we can either do color correction using it, or we can perform
> > > RGB->YCBCR conversion.
> > >
> > > This function adds a csc handler, to perform RGB->YCBCR conversion
> > > as per recommended spec values.
> >
> > Please do a full reference to the "spec", including name, version and
> > relevant
> > section.
>
> Sure, will add more details.
> > > V2: Rebase
> > > V3: Rebase
> > > V4: Rebase
> > >
> > > Cc: Ville Syrjala
> > > Cc: Daniel Vetter
> > > Cc: Ander Conselvan De Oliveira
> > > Signed-off-by: Shashank Sharma
> > > ---
> > > drivers/gpu/drm/i915/intel_color.c | 47
> > > +++-
> > > drivers/gpu/drm/i915/intel_display.c | 32
> > > 2 files changed, 78 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_color.c
> > > b/drivers/gpu/drm/i915/intel_color.c
> > > index 306c6b0..12d5f21 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -41,6 +41,19 @@
> > >
> > > #define LEGACY_LUT_LENGTH (sizeof(struct drm_color_lut) *
> > > 256)
> > >
> > > +/* Post offset values for RGB->YCBCR conversion */
> > > +#define POSTOFF_RGB_TO_YUV_HI 0x800
> > > +#define POSTOFF_RGB_TO_YUV_ME 0x100
> > > +#define POSTOFF_RGB_TO_YUV_LO 0x800
> > > +
> > > +/* Direct spec values for RGB->YUV conversion matrix */
> > > +#define CSC_RGB_TO_YUV_RU_GU 0x2ba809d8
> > > +#define CSC_RGB_TO_YUV_BU 0x37e8
> > > +#define CSC_RGB_TO_YUV_RY_GY 0x1e089cc0
> > > +#define CSC_RGB_TO_YUV_BY 0xb528
> > > +#define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
> > > +#define CSC_RGB_TO_YUV_BV 0x1e08
> > > +
> >
> > Not a big fan or hardcoding this in the register format. We already have the
> > code for converting a number to the right format for the register in
> > i915_load_csc_matrix(). I think it would make more sense to extract the code
> > that actually writes the matrix out of that function, so it would just
> > unconditionally use a matrix and coefficients passed as arguments. Then the
> > values above would be defined in the format expected for this new function.
>
> Actually I had a small discussion on this with Ville, and we think that
> the current CSC multiplication code is not correct.
> So if CTM and limited color range is applied together, we might not be
> getting the right values. So not planning to
> reuse that code. I think while sending the BT2020 patches, we will add a
> fix for that part too, but right now not working on it.
I wasn't talking about the matrix multiplication, but creating a function to
write any given matrix into the CSC. That way, the above could be hardcoded in a
more human readable format.
This issue is independent from fixing the matrix multiplication. I'm talking
specifically about the code below:
/*
* Convert fixed point S31.32 input to format supported by the
* hardware.
*/
for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i];
/*
* Clamp input value to min/max supported by
* hardware.
*/
abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);
/* sign bit */
if (CTM_COEFF_NEGATIVE(input[i]))
coeffs[i] |= 1 << 15;
if (abs_coeff < CTM_COEFF_0_125)
coeffs[i] |= (3 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 12);
else if (abs_coeff < CTM_COEFF_0_25)
coeffs[i] |= (2 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 11);
else if (abs_coeff < CTM_COEFF_0_5)
coeffs[i] |= (1 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 10);
else if (abs_coeff < CTM_COEFF_1_0)
coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9);
else if (abs_coeff < CTM_COEFF_2_0)
coeffs[i] |= (7 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 8);
else
coeffs[i] |= (6 << 12) |
I9XX_CSC_COEFF_FP(abs_coeff, 7);
Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset
On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > To get a YCBCR420 output from intel platforms, we need one
> > > scaler to scale down YCBCR444 samples to YCBCR420 samples.
> > >
> > > This patch:
> > > - Does scaler allocation for HDMI ycbcr420 outputs.
> > > - Programs PIPE_MISC register for ycbcr420 output.
> > > - Adds a new scaler user "HDMI output" to plug-into existing
> > >scaler framework. This output type is identified using bit
> > >30 of the scaler users bitmap.
> > >
> > > V2: rebase
> > > V3: rebase
> > > V4: rebase
> > >
> > > Cc: Ville Syrjala
> > > Cc: Ander Conselvan De Oliveira
> > > Signed-off-by: Shashank Sharma
> > > ---
> > > drivers/gpu/drm/i915/intel_atomic.c | 6 ++
> > > drivers/gpu/drm/i915/intel_display.c | 26 ++
> > > drivers/gpu/drm/i915/intel_drv.h | 10 +-
> > > drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
> > > drivers/gpu/drm/i915/intel_panel.c | 3 ++-
> > > 5 files changed, 53 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > > b/drivers/gpu/drm/i915/intel_atomic.c
> > > index 36d4e63..a8c9ac5 100644
> > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct
> > > drm_i915_private *dev_priv,
> > >
> > > /* panel fitter case: assign as a crtc scaler */
> > > scaler_id = &scaler_state->scaler_id;
> > > + } else if (i == SKL_HDMI_OUTPUT_INDEX) {
> > > + name = "HDMI-OUTPUT";
> > > + idx = intel_crtc->base.base.id;
> > > +
> > > + /* hdmi output case: needs a pipe scaler */
> > > + scaler_id = &scaler_state->scaler_id;
> > > } else {
> > > name = "PLANE";
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index da29536..983f581 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state
> > > *crtc_state, bool force_detach,
> > >*/
> > > need_scaling = src_w != dst_w || src_h != dst_h;
> > >
> > > + if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
> > > + && scaler_user == SKL_HDMI_OUTPUT_INDEX)
> >
> > Is it really necessary to check for both? If it is, what's the point of
> > creating
> > SKL_HDMI_OUTPUT_INDEX?
>
> Yes, else this will affect scaler update for planes, as this function
> gets called to update the plane scalers too, at that time the output
> will be still valid (as its at CRTC level), but the
> scaler user would be different.
Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
wouldn't it be a bug if hdmi_output != YCBCR420 ? Point is, if the caller asked
for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to second
guess it.
> >
> > > + /* YCBCR 444 -> 420 conversion needs a scaler */
> > > + need_scaling = true;
> > > +
> > > /*
> > >* if plane is being disabled or scaler is no more required or
> > > force detach
> > >* - free scaler binded to this plane/crtc
> > > @@ -4673,6 +4678,26 @@ skl_update_scaler(struct intel_crtc_state
> > > *crtc_state, bool force_detach,
> > > }
> > >
> > > /**
> > > + * skl_update_scaler_hdmi_output - Stages update to scaler state for
> > > HDMI.
> > > + * HDMI YCBCR 420 output needs a scaler, for downsampling.
> > > + *
> > > + * @state: crtc's scaler state
> > > + *
> > > + * Return
> > > + * 0 - scaler_usage updated successfully
> > > + *error - requested scaling cannot be supported or other error
> > > condition
> > > + */
> > > +int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state)
> > > +{
> > > + const struct drm_display_mode *mode = &state->base.adjusted_mode;
> > > +
> > > + return skl_update_scaler(state, !state->base.active,
> > > + SKL_HDMI_OUTPUT_INDEX, &state->scaler_state.scaler_id,
> > > + state->pipe_src_w, state->pipe_src_h,
> > > + mode->crtc_hdisplay, mode->crtc_vdisplay);
> > > +}
> > > +
> > > +/**
> > >* skl_update_scaler_crtc - Stages update to scaler state for a given
> > > crtc.
> > >*
> > >* @state: crtc's scaler state
> > > @@ -8058,6 +8083,7 @@ static void haswell_set_pipemisc(struct drm_crtc
> > > *crtc)
> > > {
> > > struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> > > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > > + enum drm_hdmi_output_type hdmi_out = intel_crtc->config->hdmi_output;
> > >
> > > i
[Bug 84041] Possible deadlock when using dri PRIME, caught by lockdep while playing video via vdpau
https://bugzilla.kernel.org/show_bug.cgi?id=84041 Jani Nikula (jani.nik...@intel.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |OBSOLETE --- Comment #1 from Jani Nikula (jani.nik...@intel.com) --- This bug seems to have been completely ignored, apologies. Please file a bug at https://bugs.freedesktop.org/enter_bug.cgi?product=DRI if this is still an issue with current kernels. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 06/15] drm/edid: parse sink information before CEA blocks
On Fri, Jun 30, 2017 at 10:52:54AM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 6/27/2017 5:25 PM, Ville Syrjälä wrote: > > On Wed, Jun 21, 2017 at 04:04:04PM +0530, Shashank Sharma wrote: > >> CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. > >> This block contains a map of indexes of CEA modes, which can > >> support YCBCR 420 output also. To avoid multiple parsing of same > >> CEA block, let's parse the sink information and get this map, before > >> parsing CEA modes. > >> > >> This patch moves the call to drm_add_display_info function, before the > >> mode parsing block. > >> > >> Signed-off-by: Shashank Sharma > >> --- > >> drivers/gpu/drm/drm_edid.c | 9 +++-- > >> 1 file changed, 7 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index b4583f6..42934b2 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -4605,6 +4605,13 @@ int drm_add_edid_modes(struct drm_connector > >> *connector, struct edid *edid) > >>quirks = edid_get_quirks(edid); > >> > >>/* > >> + * CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. > >> + * To avoid multiple parsing of same block, lets parse that map > >> + * from sink info, before parsing CEA modes. > >> + */ > >> + drm_add_display_info(connector, edid); > >> + > > This patch should come before the 4:2:0 mode parsing, no? > Dint you ask me to move it later (in the previous series comments), for > git-bisect regression type of changes ? I wanted it split out to help with bisecting. It should be early in the series because otherwise the rest makes no sense. And I suppose we should be able to push this in on its own right now. Just need a CI run for it, so maybe resesnd just this patch on its own. > > Otherwise I think this should be fine so > > Reviewed-by: Ville Syrjälä > Thanks. > > > >> + /* > >> * EDID spec says modes should be preferred in this order: > >> * - preferred detailed mode > >> * - other detailed modes from base block > >> @@ -4632,8 +4639,6 @@ int drm_add_edid_modes(struct drm_connector > >> *connector, struct edid *edid) > >>if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) > >>edid_fixup_preferred(connector, quirks); > >> > >> - drm_add_display_info(connector, edid); > >> - > >>if (quirks & EDID_QUIRK_FORCE_6BPC) > >>connector->display_info.bpc = 6; > >> > >> -- > >> 2.7.4 -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 01/15] drm: add HDMI 2.0 VIC support for AVI info-frames
On Wed, Jun 21, 2017 at 04:03:59PM +0530, Shashank Sharma wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.
>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch:
> - checks if the connected display is HDMI 2.0.
> - HDMI infoframes carry one of this two type of information:
> - VIC for 4K modes for HDMI 1.4 sinks
> - S3D information for S3D modes
> As CEA-861-F has already defined VICs for 4K videomodes, this
> patch doesn't allow sending HDMI infoframes for HDMI 2.0 sinks,
> until the mode is 3D.
>
> Cc: Ville Syrjala
> Cc: Jose Abreu
> Cc: Andrzej Hajda
> Cc: Alex Deucher
> Cc: Daniel Vetter
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> V2: Rebase, Added r-b from Andrzej
> V3: Addressed review comment from Ville:
> - Do not send VICs in both AVI-IF and HDMI-IF
> send only one of it.
>
> Reviewed-by: Andrzej Hajda
> Signed-off-by: Shashank Sharma
> ---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 2 +-
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 +-
> drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 ++-
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +-
> drivers/gpu/drm/drm_edid.c| 12 +++-
> drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
> drivers/gpu/drm/i2c/tda998x_drv.c | 2 +-
> drivers/gpu/drm/i915/intel_hdmi.c | 17 -
> drivers/gpu/drm/i915/intel_sdvo.c | 3 ++-
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_encoder.c| 3 ++-
> drivers/gpu/drm/radeon/radeon_audio.c | 2 +-
> drivers/gpu/drm/rockchip/inno_hdmi.c | 2 +-
> drivers/gpu/drm/sti/sti_hdmi.c| 2 +-
> drivers/gpu/drm/tegra/hdmi.c | 2 +-
> drivers/gpu/drm/tegra/sor.c | 2 +-
> drivers/gpu/drm/vc4/vc4_hdmi.c| 2 +-
> drivers/gpu/drm/zte/zx_hdmi.c | 2 +-
> include/drm/drm_edid.h| 3 ++-
> 21 files changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 3c62c45..4923ddc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1864,7 +1864,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder
> *encoder,
> dce_v10_0_audio_write_sad_regs(encoder);
> dce_v10_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index c8ed0fa..4101684 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1848,7 +1848,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder
> *encoder,
> dce_v11_0_audio_write_sad_regs(encoder);
> dce_v11_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 3e90c19..a7f6b32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1747,7 +1747,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder
> *encoder,
> dce_v8_0_audio_write_sad_regs(encoder);
> dce_v8_0_audio_write_latency_fields(encoder, mode);
>
> - err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> + err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> b/drivers/gpu/dr
Re: [RESEND-CI v4 04/15] drm/edid: parse YCBCR 420 videomodes from EDID
On Fri, Jun 30, 2017 at 10:47:48AM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 6/27/2017 5:22 PM, Ville Syrjälä wrote: > > On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote: > >> HDMI 2.0 spec adds support for YCBCR420 sub-sampled output. > >> CEA-861-F adds two new blocks in EDID's CEA extension blocks, > >> to provide information about sink's YCBCR420 output capabilities. > >> > >> These blocks are: > >> > >> - YCBCR420vdb(YCBCR 420 video data block): > >> This block contains VICs of video modes, which can be sopported only > >> in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal > >> SVD block, valid for YCBCR420 modes only. > >> > >> - YCBCR420cmdb(YCBCR 420 capability map data block): > >> This block gives information about video modes which can support > >> YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This > >> block contains a bitmap index of normal svd videomodes, which can > >> support YCBCR420 output too. > >> So if bit 0 from first vcb byte is set, first video mode in the svd > >> list can support YCBCR420 output too. Bit 1 means second video mode > >> from svd list can support YCBCR420 output too, and so on. > >> > >> This patch adds two bitmaps in display's hdmi_info structure, one each > >> for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch > >> adds: > >> - VDB modes (YCBCR 420 only modes) in connector's mode list, also makes > >>an entry in the vdb_bitmap per vic. > >> - VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap. > >> > >> Cc: Ville Syrjala > >> Cc: Jose Abreu > >> Cc: Emil Velikov > >> > >> V2: Addressed > >> Review comments from Emil: > >> - Use 1ULL< >> - Use the suggested method for updating dbmap. > >> - Add documentation for YCBCR420_vcb_map to fix kbuild warning. > >> > >> Review comments from Ville: > >> - Do not expose the YCBCR420 flags in uabi layer, keep it internal. > >> - Save a map of YCBCR420 modes for future reference. > >> - Check db length before trying to parse extended tag. > >> - Add a warning if there are > 64 modes in capability map block. > >> - Use y420cmdb in function names and macros while dealing with vcb > >>to be aligned with spec. > >> - Move the display information parsing block ahead of mode parsing > >>blocks. > >> > >> V3: Addressed design/review comments from Ville > >> - Do not add flags in video modes, else we have to expose them to user > >> - There should not be a UABI change, and kernel should detect the > >>choice of the output based on type of mode, and the bitmaps. > >> - Use standard bitops from kernel bitmap header, instead of > >> calculating > >>bit positions manually. > >> > >> V4: Addressed review comments from Ville: > >> - s/ycbcr_420_vdb/y420vdb > >> - s/ycbcr_420_vcb/y420cmdb > >> - Be less verbose on description of do_y420vdb_modes > >> - Move newmode variable in the loop scope. > >> - Use svd_to_vic() to get a VIC, instead of 0x7f > >> - Remove bitmap description for CMDB modes & VDB modes > >> - Dont add connector->ycbcr_420_allowed check for cmdb modes > >> - Remove 'len' variable, in is_y420cmdb function, which is used > >>only once > >> - Add length check in is_y420vdb function > >> - Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap > >> - Do not add print about YCBCR 420 modes > >> - Fix indentation in few places > >> - Move ycbcr420_dc_modes in next patch, where its used > >> - Add a separate patch for movement of drm_add_display_info() > >> > >> Signed-off-by: Shashank Sharma > >> --- > >> drivers/gpu/drm/drm_edid.c | 157 > >> +++- > >> include/drm/drm_connector.h | 20 ++ > >> 2 files changed, 174 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > >> index e2d1b30..8c7e73b 100644 > >> --- a/drivers/gpu/drm/drm_edid.c > >> +++ b/drivers/gpu/drm/drm_edid.c > >> @@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector, > >> struct edid *edid, > >> #define VIDEO_BLOCK 0x02 > >> #define VENDOR_BLOCK0x03 > >> #define SPEAKER_BLOCK0x04 > >> -#define VIDEO_CAPABILITY_BLOCK0x07 > >> +#define VIDEO_CAPABILITY_BLOCK 0x07 > >> +#define VIDEO_DATA_BLOCK_420 0x0E > >> +#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F > >> #define EDID_BASIC_AUDIO (1 << 6) > >> #define EDID_CEA_YCRCB444(1 << 5) > >> #define EDID_CEA_YCRCB422(1 << 4) > >> @@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct > >> drm_connector *connector, > >>return newmode; > >> } > >> > >> +/* > >> + * do_y420vdb_modes - Parse YCBCR 420 only modes > >> + * @connector: connector corresponding to the HDMI sink > >> + * @svds: start of the data block of CEA YCBCR 420 VDB > >> + * @len: l
Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset
Regards
Shashank
On 6/30/2017 5:04 PM, Ander Conselvan De Oliveira wrote:
On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
Regards
Shashank
On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
To get a YCBCR420 output from intel platforms, we need one
scaler to scale down YCBCR444 samples to YCBCR420 samples.
This patch:
- Does scaler allocation for HDMI ycbcr420 outputs.
- Programs PIPE_MISC register for ycbcr420 output.
- Adds a new scaler user "HDMI output" to plug-into existing
scaler framework. This output type is identified using bit
30 of the scaler users bitmap.
V2: rebase
V3: rebase
V4: rebase
Cc: Ville Syrjala
Cc: Ander Conselvan De Oliveira
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_atomic.c | 6 ++
drivers/gpu/drm/i915/intel_display.c | 26 ++
drivers/gpu/drm/i915/intel_drv.h | 10 +-
drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
drivers/gpu/drm/i915/intel_panel.c | 3 ++-
5 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e63..a8c9ac5 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private
*dev_priv,
/* panel fitter case: assign as a crtc scaler */
scaler_id = &scaler_state->scaler_id;
+ } else if (i == SKL_HDMI_OUTPUT_INDEX) {
+ name = "HDMI-OUTPUT";
+ idx = intel_crtc->base.base.id;
+
+ /* hdmi output case: needs a pipe scaler */
+ scaler_id = &scaler_state->scaler_id;
} else {
name = "PLANE";
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index da29536..983f581 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
*/
need_scaling = src_w != dst_w || src_h != dst_h;
+ if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
+ && scaler_user == SKL_HDMI_OUTPUT_INDEX)
Is it really necessary to check for both? If it is, what's the point of creating
SKL_HDMI_OUTPUT_INDEX?
Yes, else this will affect scaler update for planes, as this function
gets called to update the plane scalers too, at that time the output
will be still valid (as its at CRTC level), but the
scaler user would be different.
Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
wouldn't it be a bug if hdmi_output != YCBCR420 ? Point is, if the caller asked
for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to second
guess it.
skl_update_scaler function gets called for all the users, which are:
- panel fitter
- all the planes
- newly added user hdmi_output
what about the case (assume) when we have handled hdmi_output and now we
are handling for a plane, which doesnt need scaling.
in this case, the code will look like:
if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420)
need_scaling = true /* which is wrong, as this function call is
for plane scalar, and scaler_user = scaler */
if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420 &&
scaler_user == HDMI_OUT)
need_scaling = true /* this is correct, as I want to consume scalar only
when both the conditions are true.
+ /* YCBCR 444 -> 420 conversion needs a scaler */
+ need_scaling = true;
+
/*
* if plane is being disabled or scaler is no more required or force
detach
* - free scaler binded to this plane/crtc
@@ -4673,6 +4678,26 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
}
/**
+ * skl_update_scaler_hdmi_output - Stages update to scaler state for HDMI.
+ * HDMI YCBCR 420 output needs a scaler, for downsampling.
+ *
+ * @state: crtc's scaler state
+ *
+ * Return
+ * 0 - scaler_usage updated successfully
+ *error - requested scaling cannot be supported or other error condition
+ */
+int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state)
+{
+ const struct drm_display_mode *mode = &state->base.adjusted_mode;
+
+ return skl_update_scaler(state, !state->base.active,
+ SKL_HDMI_OUTPUT_INDEX, &state->scaler_state.scaler_id,
+ state->pipe_src_w, state->pipe_src_h,
+ mode->crtc_hdisplay, mode->crtc_vdisplay);
+}
+
+/**
* skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
*
* @state: crtc's scaler state
@@ -8058,6 +8083,7 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
{
struct drm_i915_private *dev_priv = to_i915(crtc-
Applied "drm: adv7511_audio: Add .get_dai_id callback to map port number to dai id" to the asoc tree
The patch
drm: adv7511_audio: Add .get_dai_id callback to map port number to dai id
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 7204e97685634813d8456f1900b7f38fa7701e60 Mon Sep 17 00:00:00 2001
From: John Stultz
Date: Tue, 13 Jun 2017 14:59:49 -0700
Subject: [PATCH] drm: adv7511_audio: Add .get_dai_id callback to map port
number to dai id
ALSA SoC needs to know connected DAI ID for probing. Using
the new audio-card-graph approach, ports/endpoints are used
to describe how the links are connected. Unfortunately, since
ports/endpoints are used as well for video linkages, there
are some issues mixing the port ids to the two (video and
audio) namespaces.
To solve this issue, this patch adds new .get_dai_id callback
on hdmi_codec_ops.
The will assume that HDMI audio out will be connected to
reg = <2>. This will then be remapped to the ALSA SoC side will
as DAI 0. Allowing the adv7511's hdmi audio support to be used
with the audio-card-graph.
Credit to Kuninori Morimoto who's patch to dw-hdmi-i2s-audio.c
was what this was mostly copy-pasted from.
Cc: Kuninori Morimoto
Cc: Archit Taneja
Cc: Mark Brown
Cc: Rob Herring
Cc: David Airlie
Cc: Lars-Peter Clausen
Cc: Linux-ALSA
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz
Signed-off-by: Mark Brown
---
.../bindings/display/bridge/adi,adv7511.txt| 8
drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 22 ++
2 files changed, 30 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
index 00ea670b8c4d..06668bca7ffc 100644
--- a/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
+++ b/Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt
@@ -78,6 +78,7 @@ graph bindings specified in
Documentation/devicetree/bindings/graph.txt.
remote endpoint phandle should be a reference to a valid mipi_dsi_host device
node.
- Video port 1 for the HDMI output
+- Audio port 2 for the HDMI audio input
Example
@@ -112,5 +113,12 @@ Example
remote-endpoint = <&hdmi_connector_in>;
};
};
+
+ port@2 {
+ reg = <2>;
+ codec_endpoint: endpoint {
+ remote-endpoint = <&i2s0_cpu_endpoint>;
+ };
+ };
};
};
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index cf92ebfe6ab7..67469c26bae8 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include "adv7511.h"
@@ -182,10 +183,31 @@ static void audio_shutdown(struct device *dev, void *data)
{
}
+static int adv7511_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
+ struct device_node *endpoint)
+{
+ struct of_endpoint of_ep;
+ int ret;
+
+ ret = of_graph_parse_endpoint(endpoint, &of_ep);
+ if (ret < 0)
+ return ret;
+
+ /*
+* HDMI sound should be located as reg = <2>
+* Then, it is sound port 0
+*/
+ if (of_ep.port == 2)
+ return 0;
+
+ return -EINVAL;
+}
+
static const struct hdmi_codec_ops adv7511_codec_ops = {
.hw_params = adv7511_hdmi_hw_params,
.audio_shutdown = audio_shutdown,
.audio_startup = audio_startup,
+ .get_dai_id = adv7511_hdmi_i2s_get_dai_id,
};
static struct hdmi_codec_pdata codec_data = {
--
2.13.2
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 06/15] drm/edid: parse sink information before CEA blocks
Regards Shashank On 6/30/2017 5:16 PM, Ville Syrjälä wrote: On Fri, Jun 30, 2017 at 10:52:54AM +0530, Sharma, Shashank wrote: Regards Shashank On 6/27/2017 5:25 PM, Ville Syrjälä wrote: On Wed, Jun 21, 2017 at 04:04:04PM +0530, Shashank Sharma wrote: CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. This block contains a map of indexes of CEA modes, which can support YCBCR 420 output also. To avoid multiple parsing of same CEA block, let's parse the sink information and get this map, before parsing CEA modes. This patch moves the call to drm_add_display_info function, before the mode parsing block. Signed-off-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index b4583f6..42934b2 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4605,6 +4605,13 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) quirks = edid_get_quirks(edid); /* +* CEA-861-F adds ycbcr capability map block, for HDMI 2.0 sinks. +* To avoid multiple parsing of same block, lets parse that map +* from sink info, before parsing CEA modes. +*/ + drm_add_display_info(connector, edid); + This patch should come before the 4:2:0 mode parsing, no? Dint you ask me to move it later (in the previous series comments), for git-bisect regression type of changes ? I wanted it split out to help with bisecting. It should be early in the series because otherwise the rest makes no sense. And I suppose we should be able to push this in on its own right now. Just need a CI run for it, so maybe resesnd just this patch on its own. Ok, got it. - Shashank Otherwise I think this should be fine so Reviewed-by: Ville Syrjälä Thanks. + /* * EDID spec says modes should be preferred in this order: * - preferred detailed mode * - other detailed modes from base block @@ -4632,8 +4639,6 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) edid_fixup_preferred(connector, quirks); - drm_add_display_info(connector, edid); - if (quirks & EDID_QUIRK_FORCE_6BPC) connector->display_info.bpc = 6; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RESEND-CI v4 15/15] drm/i915/glk: set HDMI 2.0 identifier
On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> This patch sets the is_hdmi2_src identifier in drm connector
> for GLK platform. GLK contains a native HDMI 2.0 controller.
> This identifier will help the EDID handling functions to save
> lot of work which is specific to HDMI 2.0 sources.
>
> V3: Added this patch
> V4: Rebase
>
> Signed-off-by: Shashank Sharma
This and patches 12 and 14 look fine to me. I'm not sure about the patch split,
maybe they should be squashed together in the end? And perhaps patch 10 and 13
too if the refactor I proposed are separate prep patches. But anyway, you can
use
Reviewed-by: Ander Conselvan de Oliveira
on those if you want.
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 3bd9af3..0d9d088 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1979,6 +1979,9 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> connector->doublescan_allowed = 0;
> connector->stereo_allowed = 1;
>
> + if (IS_GEMINILAKE(dev_priv))
> + connector->ycbcr_420_allowed = true;
> +
> intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
>
> switch (port) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 04/15] drm/edid: parse YCBCR 420 videomodes from EDID
Regards
Shashank
On 6/30/2017 5:28 PM, Ville Syrjälä wrote:
On Fri, Jun 30, 2017 at 10:47:48AM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 6/27/2017 5:22 PM, Ville Syrjälä wrote:
On Wed, Jun 21, 2017 at 04:04:02PM +0530, Shashank Sharma wrote:
HDMI 2.0 spec adds support for YCBCR420 sub-sampled output.
CEA-861-F adds two new blocks in EDID's CEA extension blocks,
to provide information about sink's YCBCR420 output capabilities.
These blocks are:
- YCBCR420vdb(YCBCR 420 video data block):
This block contains VICs of video modes, which can be sopported only
in YCBCR420 output mode (Not in RGB/YCBCR444/422. Its like a normal
SVD block, valid for YCBCR420 modes only.
- YCBCR420cmdb(YCBCR 420 capability map data block):
This block gives information about video modes which can support
YCBCR420 output mode also (along with RGB,YCBCR444/422 etc) This
block contains a bitmap index of normal svd videomodes, which can
support YCBCR420 output too.
So if bit 0 from first vcb byte is set, first video mode in the svd
list can support YCBCR420 output too. Bit 1 means second video mode
from svd list can support YCBCR420 output too, and so on.
This patch adds two bitmaps in display's hdmi_info structure, one each
for VCB and VDB modes. If the source is HDMI 2.0 capable, this patch
adds:
- VDB modes (YCBCR 420 only modes) in connector's mode list, also makes
an entry in the vdb_bitmap per vic.
- VCB modes (YCBCR 420 also modes) only entry in the vcb_bitmap.
Cc: Ville Syrjala
Cc: Jose Abreu
Cc: Emil Velikov
V2: Addressed
Review comments from Emil:
- Use 1ULL< 64 modes in capability map block.
- Use y420cmdb in function names and macros while dealing with vcb
to be aligned with spec.
- Move the display information parsing block ahead of mode parsing
blocks.
V3: Addressed design/review comments from Ville
- Do not add flags in video modes, else we have to expose them to user
- There should not be a UABI change, and kernel should detect the
choice of the output based on type of mode, and the bitmaps.
- Use standard bitops from kernel bitmap header, instead of calculating
bit positions manually.
V4: Addressed review comments from Ville:
- s/ycbcr_420_vdb/y420vdb
- s/ycbcr_420_vcb/y420cmdb
- Be less verbose on description of do_y420vdb_modes
- Move newmode variable in the loop scope.
- Use svd_to_vic() to get a VIC, instead of 0x7f
- Remove bitmap description for CMDB modes & VDB modes
- Dont add connector->ycbcr_420_allowed check for cmdb modes
- Remove 'len' variable, in is_y420cmdb function, which is used
only once
- Add length check in is_y420vdb function
- Remove unnecessary if (!db) check in function parse_y420cmdb_bitmap
- Do not add print about YCBCR 420 modes
- Fix indentation in few places
- Move ycbcr420_dc_modes in next patch, where its used
- Add a separate patch for movement of drm_add_display_info()
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/drm_edid.c | 157
+++-
include/drm/drm_connector.h | 20 ++
2 files changed, 174 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e2d1b30..8c7e73b 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2781,7 +2781,9 @@ add_detailed_modes(struct drm_connector *connector,
struct edid *edid,
#define VIDEO_BLOCK 0x02
#define VENDOR_BLOCK0x03
#define SPEAKER_BLOCK0x04
-#define VIDEO_CAPABILITY_BLOCK 0x07
+#define VIDEO_CAPABILITY_BLOCK 0x07
+#define VIDEO_DATA_BLOCK_420 0x0E
+#define VIDEO_CAP_BLOCK_Y420CMDB 0x0F
#define EDID_BASIC_AUDIO (1 << 6)
#define EDID_CEA_YCRCB444(1 << 5)
#define EDID_CEA_YCRCB422(1 << 4)
@@ -3153,15 +3155,79 @@ drm_display_mode_from_vic_index(struct drm_connector
*connector,
return newmode;
}
+/*
+ * do_y420vdb_modes - Parse YCBCR 420 only modes
+ * @connector: connector corresponding to the HDMI sink
+ * @svds: start of the data block of CEA YCBCR 420 VDB
+ * @len: length of the CEA YCBCR 420 VDB
+ *
+ * Parse the CEA-861-F YCBCR 420 Video Data Block (Y420VDB)
+ * which contains modes which can be supported in YCBCR 420
+ * output format only.
+ */
+static int do_y420vdb_modes(struct drm_connector *connector,
+ const u8 *svds, u8 svds_len)
+{
+ int modes = 0, i;
+ struct drm_device *dev = connector->dev;
+ struct drm_display_info *info = &connector->display_info;
+ struct drm_hdmi_info *hdmi = &info->hdmi;
+
+ for (i = 0; i < svds_len; i++) {
+ u8 vic = svd_to_vic(svds[i]);
+ struct drm_display_mode *newmode;
+
+ newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]);
+ if (!newmode)
+ break;
+ bitmap_s
Re: [Intel-gfx] [RESEND-CI v4 15/15] drm/i915/glk: set HDMI 2.0 identifier
Regards
Shashank
On 6/30/2017 5:37 PM, Ander Conselvan De Oliveira wrote:
On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
This patch sets the is_hdmi2_src identifier in drm connector
for GLK platform. GLK contains a native HDMI 2.0 controller.
This identifier will help the EDID handling functions to save
lot of work which is specific to HDMI 2.0 sources.
V3: Added this patch
V4: Rebase
Signed-off-by: Shashank Sharma
This and patches 12 and 14 look fine to me. I'm not sure about the patch split,
maybe they should be squashed together in the end? And perhaps patch 10 and 13
too if the refactor I proposed are separate prep patches.
In fact this is exactly how I prepared at the first place, keeping all
the crtc/pipe level changes together.
But then I thought the patch might be touching too many things, and
might be too big or complex for
review, that's why I had split into 3-4 small patches :-)
But anyway, you can
use
Reviewed-by: Ander Conselvan de Oliveira
on those if you want.
Thanks, I guess this applies for 12,14,10,13 and 15 (with a separate
patch for CSC coeff handling).
Please correct me if I misunderstood.
- Shashank
---
drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 3bd9af3..0d9d088 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1979,6 +1979,9 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
connector->doublescan_allowed = 0;
connector->stereo_allowed = 1;
+ if (IS_GEMINILAKE(dev_priv))
+ connector->ycbcr_420_allowed = true;
+
intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
switch (port) {
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vblank: Fix race between drm_crtc_arm_vblank_event() and the irq
From: Ville Syrjälä Make sure the armed even doesn't fire until the next vblank by adding one to the current vblank counter value. This will prevent the event being fired off prematurely if drm_handle_vblank() is called redundantly, or if the irq handler gets delayed somehow. This is actually a real race on Intel gen2/3 hardware due to the vblank interrupt firing approx. one or more scanlines after the start of vblank (which is when the double buffered registers get latched). So if we were to perform an atomic update just after the start of vblank, but before the irq has fired, the irq handler would send out the the event immediately instead of waiting for the next vblank like it should. This also makes logs less confusing because now it normally looks like the vblank interrupt was somehow missed and the event gets sent one frame too late. Cc: Daniel Vetter Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_vblank.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index bfe86fa2d3b1..823c853de052 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, assert_spin_locked(&dev->event_lock); e->pipe = pipe; - e->event.sequence = drm_vblank_count(dev, pipe); + e->event.sequence = drm_vblank_count(dev, pipe) + 1; e->event.crtc_id = crtc->base.id; list_add_tail(&e->base.link, &dev->vblank_event_list); } -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [BISECTED, REGRESSION] v4.12-rc: omapdrm fails to probe on Nokia N900
On Fri, Jun 30, 2017 at 09:41:35AM +0300, Tomi Valkeinen wrote: > On 29/06/17 21:50, Aaro Koskinen wrote: > > Hi, > > > > On Thu, Jun 15, 2017 at 09:51:06AM +0300, Tomi Valkeinen wrote: > >> On 15/06/17 01:11, Aaro Koskinen wrote: > >>> When booting v4.12-rc5 on Nokia N900, omapdrm fails to probe and there > >>> is no display. > >> > >> Are you sure it doesn't probe? It fails the omapdss_stack_is_ready() > >> check? > > > > It appears the reason was that I didn't have > > CONFIG_DRM_OMAP_CONNECTOR_ANALOG_TV enabled. > > > > I think that's wrong. I don't own an analog TV, so why should I enable > > such option to get device's built-in display working? > > Indeed. Unfortunately I don't have a solution for that. > > DRM doesn't support adding devices after probe. So at omapdrm probe time > we have to decide which displays to use. In the dts file, n900 defines > the lcd and analog tv. omapdrm sees those, and, of course, must wait > until their respective drivers have probed. If you don't have the > display driver enabled, it's never loaded and omapdrm never probes as it > keeps waiting for those. > > omapdrm could start when some of the drivers are missing, but then the > question comes: when? omapdrm doesn't know if the driver will be probed > at some later time, or maybe it is a module, loaded later by the userspace. > > So, unless the DRM framework is modified to support adding displays > after probe, I don't see a solution for this. > > >> If that's the case then this is easier to debug. > > > > Sure it's always easy... when users do all the testing and debugging. > > > > Is it just me or do other OMAP users fail to see omapdrm changes being > > posted to linux-omap for testing or review purposes? > > I thought linux-omap was more for omap platform thingies. But sure, I > can start cc'ing linux-omap for all omapdrm patches. > > > And now I face another issue. When I boot v4.12-rc7 with > > CONFIG_DRM_OMAP_CONNECTOR_ANALOG_TV enabled and your "misc fixes" > > patches on N900, I get the error flood again, and the device is unusable: > > Yes, I think there's still something wrong. I haven't been able to find > out what. > > Somehow delays seem to help (like enabling debug prints), but I haven't > been able to find out where exactly the delay is needed. > > And I'd rather not revert the patch, because there's nothing wrong with > that patch as such, and it fixes issues on all platforms. > > So, I don't know... I guess I need to try to invent some horrible hacks > around the driver to somehow manage the omap3 problems. Perhaps > disabling/enabling the outputs when sync lost happens... I don't think registering before everything is loaded make sense. On the big desktop driver chips we have all the bridge/encoder/panel drivers built into the driver. arm-soc loves to make everything a separate module, but in the end if you decided to not compile half of the driver you need, then it's not going to work. Imo the only thing we should support to be hotplugged in drm is stuff you can physically hotplug (like atm connectors). Everything else just complicates the code for no good reason at all. tldr; Wrong .config gives you a non-working driver, no surprises. And due to the "default n" rule this can even look like a regression. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/amdgpu: Set/clear CPU_ACCESS_REQUIRED flag on page fault and CS
On Fri, Jun 30, 2017 at 08:47:27AM +0200, Christian König wrote: > Am 30.06.2017 um 04:24 schrieb Michel Dänzer: > > On 29/06/17 07:05 PM, Daniel Vetter wrote: > > > On Thu, Jun 29, 2017 at 06:58:05PM +0900, Michel Dänzer wrote: > > > > On 29/06/17 05:23 PM, Christian König wrote: > > > > > Am 29.06.2017 um 04:35 schrieb Michel Dänzer: > > > > > > On 29/06/17 08:26 AM, John Brooks wrote: > > > > > > > On Wed, Jun 28, 2017 at 03:05:32PM +0200, Christian König wrote: > > > > > > > > > Instead of the flag being set in stone at BO creation, set > > > > > > > > > the flag > > > > > > > > > when a > > > > > > > > > page fault occurs so that it goes somewhere CPU-visible, and > > > > > > > > > clear > > > > > > > > > it when > > > > > > > > > the BO is requested by the GPU. > > > > > > > > > > > > > > > > > > However, clearing the CPU_ACCESS_REQUIRED flag may move BOs > > > > > > > > > in GTT to > > > > > > > > > invisible VRAM, where they may promptly generate another page > > > > > > > > > fault. When > > > > > > > > > BOs are constantly moved back and forth like this, it is > > > > > > > > > highly > > > > > > > > > detrimental > > > > > > > > > to performance. Only clear the flag on CS if: > > > > > > > > > > > > > > > > > > - The BO wasn't page faulted for a certain amount of time > > > > > > > > > (currently 10 > > > > > > > > > seconds), and > > > > > > > > > - its last page fault didn't occur too soon (currently 500ms) > > > > > > > > > after > > > > > > > > > its > > > > > > > > > last CS request, or vice versa. > > > > > > > > > > > > > > > > > > Setting the flag in amdgpu_fault_reserve_notify() also means > > > > > > > > > that > > > > > > > > > we can > > > > > > > > > remove the loop to restrict lpfn to the end of visible VRAM, > > > > > > > > > because > > > > > > > > > amdgpu_ttm_placement_init() will do it for us. > > > > > > > > I'm fine with the general approach, but I'm still absolutely not > > > > > > > > keen about > > > > > > > > clearing the flag when userspace has originally specified it. > > > > > > Is there any specific concern you have about that? > > > > > Yeah, quite a bunch actually. We want to use this flag for P2P buffer > > > > > sharing in the future as well and I don't intent to add another one > > > > > like > > > > > CPU_ACCESS_REALLY_REQUIRED or something like this. > > > > Won't a BO need to be pinned while it's being shared with another > > > > device? > > > That's an artifact of the current kernel implementation, I think we could > > > do better (but for current use-cases where we share a bunch of scanouts > > > and maybe a few pixmaps it's pointless). I wouldn't bet uapi on this never > > > changing. > > Surely there will need to be some kind of transaction though to let the > > driver know when a BO starts/stops being shared with another device? > > Either via the existing dma-buf callbacks, or something similar. We > > can't rely on userspace setting a "CPU access" flag to make sure a BO > > can be shared with other devices? Well I just jumped into the middle of this that it's not entirely out of the question as an idea, but yeah we'd need to rework the dma-buf stuff with probably a callback to evict mappings/stall for outstanding rendering or something like that. > Well, the flag was never intended to be used by userspace. > > See the history was more like we need something in the kernel to place the > BO in CPU accessible VRAM. > > Then the closed source UMD came along and said hey we have the concept of > two different heaps for visible and invisible VRAM, how does that maps to > amdgpu? > > I unfortunately was to tired to push back hard enough on this Ehrm, are you saying you have uapi for the closed source stack only? I can help with the push back on this with a revert, no problem :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/atomic: Add missing drm_atomic_state_clear to atomic_remove_fb
On Thu, Jun 29, 2017 at 05:17:39PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 29, 2017 at 04:57:25PM +0300, Ville Syrjälä wrote:
> > On Thu, Jun 29, 2017 at 01:59:54PM +0200, Maarten Lankhorst wrote:
> > > Signed-off-by: Maarten Lankhorst
> > > ---
> > > drivers/gpu/drm/drm_framebuffer.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_framebuffer.c
> > > b/drivers/gpu/drm/drm_framebuffer.c
> > > index fc8ef42203ec..b3ef4f1c2630 100644
> > > --- a/drivers/gpu/drm/drm_framebuffer.c
> > > +++ b/drivers/gpu/drm/drm_framebuffer.c
> > > @@ -832,6 +832,7 @@ static int atomic_remove_fb(struct drm_framebuffer
> > > *fb)
> > > drm_atomic_clean_old_fb(dev, plane_mask, ret);
> > >
> > > if (ret == -EDEADLK) {
> > > + drm_atomic_state_clear(state);
> >
> > Hmm. We seem to be missing this all over. Do those other places need it
> > as well? Hard to say without a commit message explaining why we need it
> > here.
> >
> > Should we just back it into drm_modeset_backoff() if it's always needed?
>
> s/back/bake/
It's not needed everywhere, and that's because you can do the modeset_lock
dance without necessarily doing an atomic transaction (e.g. hw state
readout on boot or load detect). Or the atomic transaction is happening
somewhere else from where the ctx backoff is handled (e.g. for legacy
paths the core code handles the w/w dance since my recent rework, whereas
compat helpers handle the retry for the atomic side).
But yeah if you do an atomic commit, you must have the state_clear in the
EDEADLK path somewhere. Maybe we could do a trick with lockdep annotations
(make the state another ww mutex that nests within the modeset_lock class,
or something like that) to ensure that no one ever forgets to clear this
up? But that's a bit tricky, since on successful commit we hand the state
over to the driver and must _not_ clear it, this is only for the backoff
case (even on any other errors it's not needed, since we just kfree
everything).
-Daniel
>
> >
> > > drm_modeset_backoff(&ctx);
> > > goto retry;
> > > }
> > > --
> > > 2.11.0
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/amdgpu: Set/clear CPU_ACCESS_REQUIRED flag on page fault and CS
Am 30.06.2017 um 14:39 schrieb Daniel Vetter: On Fri, Jun 30, 2017 at 08:47:27AM +0200, Christian König wrote: Am 30.06.2017 um 04:24 schrieb Michel Dänzer: On 29/06/17 07:05 PM, Daniel Vetter wrote: On Thu, Jun 29, 2017 at 06:58:05PM +0900, Michel Dänzer wrote: On 29/06/17 05:23 PM, Christian König wrote: Am 29.06.2017 um 04:35 schrieb Michel Dänzer: On 29/06/17 08:26 AM, John Brooks wrote: On Wed, Jun 28, 2017 at 03:05:32PM +0200, Christian König wrote: Instead of the flag being set in stone at BO creation, set the flag when a page fault occurs so that it goes somewhere CPU-visible, and clear it when the BO is requested by the GPU. However, clearing the CPU_ACCESS_REQUIRED flag may move BOs in GTT to invisible VRAM, where they may promptly generate another page fault. When BOs are constantly moved back and forth like this, it is highly detrimental to performance. Only clear the flag on CS if: - The BO wasn't page faulted for a certain amount of time (currently 10 seconds), and - its last page fault didn't occur too soon (currently 500ms) after its last CS request, or vice versa. Setting the flag in amdgpu_fault_reserve_notify() also means that we can remove the loop to restrict lpfn to the end of visible VRAM, because amdgpu_ttm_placement_init() will do it for us. I'm fine with the general approach, but I'm still absolutely not keen about clearing the flag when userspace has originally specified it. Is there any specific concern you have about that? Yeah, quite a bunch actually. We want to use this flag for P2P buffer sharing in the future as well and I don't intent to add another one like CPU_ACCESS_REALLY_REQUIRED or something like this. Won't a BO need to be pinned while it's being shared with another device? That's an artifact of the current kernel implementation, I think we could do better (but for current use-cases where we share a bunch of scanouts and maybe a few pixmaps it's pointless). I wouldn't bet uapi on this never changing. Surely there will need to be some kind of transaction though to let the driver know when a BO starts/stops being shared with another device? Either via the existing dma-buf callbacks, or something similar. We can't rely on userspace setting a "CPU access" flag to make sure a BO can be shared with other devices? Well I just jumped into the middle of this that it's not entirely out of the question as an idea, but yeah we'd need to rework the dma-buf stuff with probably a callback to evict mappings/stall for outstanding rendering or something like that. Well, the flag was never intended to be used by userspace. See the history was more like we need something in the kernel to place the BO in CPU accessible VRAM. Then the closed source UMD came along and said hey we have the concept of two different heaps for visible and invisible VRAM, how does that maps to amdgpu? I unfortunately was to tired to push back hard enough on this Ehrm, are you saying you have uapi for the closed source stack only? No, Mesa is using that flag as well. What I'm saying is that we have a flag which became uapi because I was to lazy to distinct between uapi and kernel internal flags. I can help with the push back on this with a revert, no problem :-) That would break Mesa and is not an option (unfortunately :). Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/atomic: Add missing drm_atomic_state_clear to atomic_remove_fb
On Thu, Jun 29, 2017 at 01:59:54PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst
With a real commit message (just explain why it's needed):
Reviewed-by: Daniel Vetter
Also needs
Fixes: db8f6403e88a ("drm: Convert drm_framebuffer_remove to atomic, v4.")
Cc: sta...@vger.kernel.org # v4.12-rc1+
Please push to drm-misc-next-fixes so Sean can send a pull for it for
4.13.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_framebuffer.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index fc8ef42203ec..b3ef4f1c2630 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -832,6 +832,7 @@ static int atomic_remove_fb(struct drm_framebuffer *fb)
> drm_atomic_clean_old_fb(dev, plane_mask, ret);
>
> if (ret == -EDEADLK) {
> + drm_atomic_state_clear(state);
> drm_modeset_backoff(&ctx);
> goto retry;
> }
> --
> 2.11.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm: Convert atomic drivers from CRTC .disable() to .atomic_disable()
On Fri, Jun 30, 2017 at 12:36:45PM +0300, Laurent Pinchart wrote:
> The CRTC .disable() helper operation is deprecated for atomic drivers,
> the new .atomic_disable() helper operation being preferred. Convert all
> atomic drivers to .atomic_disable() to avoid cargo-cult use of
> .disable() in new drivers.
>
> Signed-off-by: Laurent Pinchart
> Acked-by: Maxime Ripard # for sun4i
> Acked-by: Philipp Zabel # for mediatek
> Acked-by: Alexey Brodkin # for arcpgu
> Acked-by: Boris Brezillon # for
> atmel-hlcdc
> Tested-by: Philippe Cornu # for stm
> Acked-by: Philippe Cornu # for stm
> Acked-by: Vincent Abriou # for sti
> Reviewed-by: Thomas Hellstrom # for vmwgfx
Patches 1&2 merged, I think they gathered sufficient acks :-)
I'll leave 3 for Thomas to pick up for 4.14 through the vmwgfx tree.
Thanks, Daniel
> ---
> drivers/gpu/drm/arc/arcpgu_crtc.c | 5 +++--
> drivers/gpu/drm/arm/hdlcd_crtc.c| 5 +++--
> drivers/gpu/drm/arm/malidp_crtc.c | 5 +++--
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 5 +++--
> drivers/gpu/drm/drm_simple_kms_helper.c | 5 +++--
> drivers/gpu/drm/exynos/exynos_drm_crtc.c| 5 +++--
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 5 +++--
> drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 5 +++--
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 +++--
> drivers/gpu/drm/meson/meson_crtc.c | 5 +++--
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c| 5 +++--
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c| 5 +++--
> drivers/gpu/drm/omapdrm/omap_crtc.c | 5 +++--
> drivers/gpu/drm/qxl/qxl_display.c | 5 +++--
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 5 +++--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 5 +++--
> drivers/gpu/drm/sti/sti_crtc.c | 5 +++--
> drivers/gpu/drm/stm/ltdc.c | 5 +++--
> drivers/gpu/drm/sun4i/sun4i_crtc.c | 5 +++--
> drivers/gpu/drm/tegra/dc.c | 5 +++--
> drivers/gpu/drm/tilcdc/tilcdc_crtc.c| 8 +++-
> drivers/gpu/drm/vc4/vc4_crtc.c | 5 +++--
> drivers/gpu/drm/virtio/virtgpu_display.c| 5 +++--
> drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 7 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 7 ---
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 5 +++--
> drivers/gpu/drm/zte/zx_vou.c| 5 +++--
> 27 files changed, 87 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c
> b/drivers/gpu/drm/arc/arcpgu_crtc.c
> index c9bc6a90ac83..1859dd3ad622 100644
> --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> @@ -130,7 +130,8 @@ static void arc_pgu_crtc_atomic_enable(struct drm_crtc
> *crtc,
> ARCPGU_CTRL_ENABLE_MASK);
> }
>
> -static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
> +static void arc_pgu_crtc_atomic_disable(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> {
> struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
>
> @@ -162,9 +163,9 @@ static const struct drm_crtc_helper_funcs
> arc_pgu_crtc_helper_funcs = {
> .mode_set = drm_helper_crtc_mode_set,
> .mode_set_base = drm_helper_crtc_mode_set_base,
> .mode_set_nofb = arc_pgu_crtc_mode_set_nofb,
> - .disable= arc_pgu_crtc_disable,
> .atomic_begin = arc_pgu_crtc_atomic_begin,
> .atomic_enable = arc_pgu_crtc_atomic_enable,
> + .atomic_disable = arc_pgu_crtc_atomic_disable,
> };
>
> static void arc_pgu_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c
> b/drivers/gpu/drm/arm/hdlcd_crtc.c
> index 2b7f4f05d91f..16e1e20cf04c 100644
> --- a/drivers/gpu/drm/arm/hdlcd_crtc.c
> +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
> @@ -176,7 +176,8 @@ static void hdlcd_crtc_atomic_enable(struct drm_crtc
> *crtc,
> drm_crtc_vblank_on(crtc);
> }
>
> -static void hdlcd_crtc_disable(struct drm_crtc *crtc)
> +static void hdlcd_crtc_atomic_disable(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> {
> struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
>
> @@ -219,10 +220,10 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc
> *crtc,
> }
>
> static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
> - .disable= hdlcd_crtc_disable,
> .atomic_check = hdlcd_crtc_atomic_check,
> .atomic_begin = hdlcd_crtc_atomic_begin,
> .atomic_enable = hdlcd_crtc_atomic_enable,
> + .atomic_disable = hdlcd_crtc_atomic_disable,
> };
>
> static int hdlcd_plane_atomic_check(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index 8e5b1c0181ab..3615d18a7ddf 100644
> --- a/drivers/gpu/drm/arm/malidp_c
Re: Resizeable PCI BAR support V5
Hi Dieter, thanks a lot for testing that. But I think my poor little FUJITSU PRIMERGY TX150 S7, Xeon X3470 (Nehalem), PCIe 2.0, 24 GB is to old for this stuff... Well, actually you only need to figure out how to enable a PCIe window above the 4GB limit. Could be that the BIOS supports this with the ACPI tables (totally unlikely) or you could try to dig up the Northbridge documentation for this CPU from Intel and use my patch for the AMD CPUs as blueprint how to do this on an Intel CPU as well. Fact is you GFX hardware is perfectly capable of doing this, it's just that the BIOS/Motherboard didn't enabled a PCIe window per default to avoid problems with 32bit OSes. Regards, Christian. Am 30.06.2017 um 01:51 schrieb Dieter Nützel: Hello Christian, I've running this since you've sent it on-top of amd-staging-4.11. But I think my poor little FUJITSU PRIMERGY TX150 S7, Xeon X3470 (Nehalem), PCIe 2.0, 24 GB is to old for this stuff... [1.066475] pci :05:00.0: VF(n) BAR0 space: [mem 0x-0x0003 64bit] (contains BAR0 for 16 VFs) [1.066489] pci :05:00.0: VF(n) BAR2 space: [mem 0x-0x003f 64bit] (contains BAR2 for 16 VFs) [1.121656] pci :00:1c.0: BAR 15: assigned [mem 0x8000-0x801f 64bit pref] [1.121659] pci :00:1c.6: BAR 15: assigned [mem 0x8020-0x803f 64bit pref] [1.121662] pci :01:00.0: BAR 6: assigned [mem 0xb012-0xb013 pref] [1.121681] pci :05:00.0: BAR 6: assigned [mem 0xb028-0xb02f pref] [1.121683] pci :05:00.0: BAR 9: no space for [mem size 0x0040 64bit] [1.121684] pci :05:00.0: BAR 9: failed to assign [mem size 0x0040 64bit] [1.121685] pci :05:00.0: BAR 7: no space for [mem size 0x0004 64bit] [1.121687] pci :05:00.0: BAR 7: failed to assign [mem size 0x0004 64bit] [3.874180] amdgpu :01:00.0: BAR 0: releasing [mem 0xc000-0xcfff 64bit pref] [3.874182] amdgpu :01:00.0: BAR 2: releasing [mem 0xb040-0xb05f 64bit pref] [3.874198] pcieport :00:03.0: BAR 15: releasing [mem 0xb040-0xcfff 64bit pref] [3.874215] pcieport :00:03.0: BAR 15: no space for [mem size 0x3 64bit pref] [3.874217] pcieport :00:03.0: BAR 15: failed to assign [mem size 0x3 64bit pref] [3.874221] amdgpu :01:00.0: BAR 0: no space for [mem size 0x2 64bit pref] [3.874223] amdgpu :01:00.0: BAR 0: failed to assign [mem size 0x2 64bit pref] [3.874226] amdgpu :01:00.0: BAR 2: no space for [mem size 0x0020 64bit pref] [3.874227] amdgpu :01:00.0: BAR 2: failed to assign [mem size 0x0020 64bit pref] [3.874258] [drm] Not enough PCI address space for a large BAR. [3.874261] amdgpu :01:00.0: BAR 0: assigned [mem 0xc000-0xcfff 64bit pref] [3.874269] amdgpu :01:00.0: BAR 2: assigned [mem 0xb040-0xb05f 64bit pref] [3.874288] [drm] Detected VRAM RAM=8192M, BAR=256M Anyway rebase for current amd-staging-4.11 needed. Find attached dmesg-amd-staging-4.11-1.g7262353-default+.log.xz Greetings, Dieter Am 09.06.2017 10:59, schrieb Christian König: Hi everyone, This is the fith incarnation of this set of patches. It enables device drivers to resize and most likely also relocate the PCI BAR of devices they manage to allow the CPU to access all of the device local memory at once. This is very useful for GFX device drivers where the default PCI BAR is only about 256MB in size for compatibility reasons, but the device easily have multiple gigabyte of local memory. Some changes since V4: 1. Rebased on 4.11. 2. added the rb from Andy Shevchenko to patches which look complete now. 3. Move releasing the BAR and reallocating it on error to the driver side. 4. Add amdgpu support for GMC V6 hardware generation as well. Please review and/or comment, Christian. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: Drop helper include from drm_atomic.c
Hi Daniel, 2017-06-30 Daniel Vetter : > Core code should never have to look at helper stuff, to make sure that > all helper code is 100% optional and can be overriden. > > Cc: Gustavo Padovan > Signed-off-by: Daniel Vetter Reviewed-by: Gustavo Padovan Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vblank: Fix race between drm_crtc_arm_vblank_event() and the irq
On Fri, Jun 30, 2017 at 03:27:29PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Make sure the armed even doesn't fire until the next vblank by adding > one to the current vblank counter value. This will prevent the event > being fired off prematurely if drm_handle_vblank() is called > redundantly, or if the irq handler gets delayed somehow. > > This is actually a real race on Intel gen2/3 hardware due to the vblank > interrupt firing approx. one or more scanlines after the start of vblank > (which is when the double buffered registers get latched). So if we were > to perform an atomic update just after the start of vblank, but before > the irq has fired, the irq handler would send out the the event immediately > instead of waiting for the next vblank like it should. > > This also makes logs less confusing because now it normally looks like > the vblank interrupt was somehow missed and the event gets sent one > frame too late. So this para here I like, since the others are already written off in the kernel-doc as "don't use this function if this is a problem for you". I'd say your claim is even wrong, since we're not using drm_accurate_vblank_count, and so the race still exists (if you're sufficiently unlucky at least, and since there can be an arbitrary delay between when the hw raises an irq and when the kernel runs the irq handler it's not any better). Anyway, as long as it's not cc: stable (that would need more convincing on my side): Reviewed-by: Daniel Vetter > > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/drm_vblank.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index bfe86fa2d3b1..823c853de052 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, > assert_spin_locked(&dev->event_lock); > > e->pipe = pipe; > - e->event.sequence = drm_vblank_count(dev, pipe); > + e->event.sequence = drm_vblank_count(dev, pipe) + 1; > e->event.crtc_id = crtc->base.id; > list_add_tail(&e->base.link, &dev->vblank_event_list); > } > -- > 2.13.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/5] drm/atomic: Refactor drm_atomic_state_realloc_connectors()
On Thu, Jun 29, 2017 at 04:49:44PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Pull the code to reallocate the state->connectors[] array into a
> helper function. We'll have another use for this later.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_atomic.c | 43 +--
> include/drm/drm_atomic.h | 5 +
> 2 files changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 095e87278a88..a9f02b214fc6 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1043,6 +1043,32 @@ drm_atomic_get_private_obj_state(struct
> drm_atomic_state *state, void *obj,
> }
> EXPORT_SYMBOL(drm_atomic_get_private_obj_state);
>
> +int drm_atomic_state_realloc_connectors(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + int index)
Needs some pretty kerneldoc, best with some explanation why/when drivers
might want to use this (i.e. when they're constructing their own state for
special reasons like hw state read-out or recovery after a hw reset or
whatever). Commit message should explain that too, but better to stuff it
into the kerneldoc. With that addressed:
Reviewed-by: Daniel Vetter
> +{
> + struct drm_mode_config *config = &dev->mode_config;
> + struct __drm_connnectors_state *c;
> + int alloc = max(index + 1, config->num_connector);
> +
> + if (index < state->num_connector)
> + return 0;
> +
> + c = krealloc(state->connectors,
> + alloc * sizeof(*state->connectors), GFP_KERNEL);
> + if (!c)
> + return -ENOMEM;
> +
> + state->connectors = c;
> + memset(&state->connectors[state->num_connector], 0,
> +sizeof(*state->connectors) * (alloc - state->num_connector));
> +
> + state->num_connector = alloc;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_state_realloc_connectors);
> +
> /**
> * drm_atomic_get_connector_state - get connector state
> * @state: global atomic state object
> @@ -1074,20 +1100,9 @@ drm_atomic_get_connector_state(struct drm_atomic_state
> *state,
>
> index = drm_connector_index(connector);
>
> - if (index >= state->num_connector) {
> - struct __drm_connnectors_state *c;
> - int alloc = max(index + 1, config->num_connector);
> -
> - c = krealloc(state->connectors, alloc *
> sizeof(*state->connectors), GFP_KERNEL);
> - if (!c)
> - return ERR_PTR(-ENOMEM);
> -
> - state->connectors = c;
> - memset(&state->connectors[state->num_connector], 0,
> -sizeof(*state->connectors) * (alloc -
> state->num_connector));
> -
> - state->num_connector = alloc;
> - }
> + ret = drm_atomic_state_realloc_connectors(connector->dev, state, index);
> + if (ret)
> + return ERR_PTR(ret);
>
> if (state->connectors[index].state)
> return state->connectors[index].state;
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 0196f264a418..5596ad58bcdc 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -324,6 +324,11 @@ drm_atomic_get_private_obj_state(struct drm_atomic_state
> *state,
> void *obj,
> const struct drm_private_state_funcs *funcs);
>
> +int __must_check
> +drm_atomic_state_realloc_connectors(struct drm_device *dev,
> + struct drm_atomic_state *state,
> + int index);
> +
> /**
> * drm_atomic_get_existing_crtc_state - get crtc state, if it exists
> * @state: global atomic state object
> --
> 2.13.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Thu, Jun 29, 2017 at 04:49:48PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Introduce an rw_semaphore to protect the display commits. All normal
> commits use down_read() and hence can proceed in parallel, but GPU reset
> will use down_write() making sure no other commits are in progress when
> we have to pull the plug on the display engine on pre-g4x platforms.
> There are no modeset/gem locks taken inside __intel_atomic_commit_tail()
> itself, and we wait for all dependencies before the down_read(), and
> thus there is no chance of deadlocks with this scheme.
How does this solve the deadlock? Afaiui the deadlock is that the gpu
reset stopped unconditionally completing all requests before it did
anything else, including anything with the hw or taking modeset locks.
This ensured that any outstanding flips (we only had pageflips, no atomic
ones back then) could complete (although maybe displaying garbage). The
only thing we had to do was grab the locks (to avoid concurrent modesets)
and then we could happily nuke the hw (since the flips where lost in the
CS anyway), and restore it afterwards.
Then the TDR rewrite came around and broke that, which now makes atomic
time out waiting for the gpu to complete (because the gpu reset waits for
the modeset to complete first). Which means if you want to fix this
without breaking TDR, then you need to cancel the pending atomic commits.
That seems somewhat what you're attempting here with trying to figure out
what the latest hw-committed step is (but that function gets it wrong),
and lots more locking and stuff on top.
Why exactly can't we just go back to the old world of force-completing
dead requests on gen4 and earlier? That would be tons simpler imo instead
of throwing a pile of hacks (which really needs a complete rewrite of the
atomic code in i915) in as a work-around. We didn't have TDR on gen4 and
earlier for years, going back to that isn't going to hurt anyone.
Making working gen4 gpu reset contigent on cancellable atomic commits and
the full commit queue is imo nuts.
-Daniel
>
> During reset we should be recommiting the state that was committed last.
> But for now we'll settle for recommiting the last state for each object.
> Hence we may commit things a bit too soon when a GPU reset occurs. The
> rw_semaphore should guarantee that whatever state we observe in
> obj->state during reset sticks around while we do the commit. The
> obj->state pointer might change for some objects if another swap_state()
> occurs while we do our thing, so there migth be some theoretical
> mismatch which I tink we could avoid by grabbing the rw_semaphore also
> around the swap_state(), but for now I didn't do that.
>
> Another source of mismatch can happen because we sometimes use the
> intel_crtc->config during the actual commit, and that only gets updated
> when we do the commuit. Hence we may get some state via ->state, some
> via the duplicated ->state, and some via ->config. We'll want to
> fix this all by tracking the commited state properly, but that will
> some bigger refactroing so for now we'll just choose to accept these
> potential mismatches.
>
> I left out the state readout from the post-reset display
> reinitialization as that still likes to clobber crtc->state etc.
> If we make it use a free standing atomic state and mke sure it doesn't
> need any locks we could reintroduce it. For now I just mark the
> post-reset display state as dirty as possible to make sure we
> reinitialize everything.
>
> One other potential issue remains in the form of display detection.
> Either we need to protect that with the same rw_semaphore as well, or
> perhaps it would be enough to force gmbus into bitbanging mode while
> the reset is happening and we don't have interrupts, and just across
> the actual hardware GPU reset we could hold the gmbus mutex.
>
> v2: Keep intel_prepare/finish_reset() outside struct_mutex (Chris)
> v3: Drop all the committed_state refactoring to make this less
> obnoxious to backport (Daniel)
>
> Cc: sta...@vger.kernel.org # for the brave
> Cc: Daniel Vetter
> Cc: Chris Wilson
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101597
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99093
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101600
> Fixes: 4680816be336 ("drm/i915: Wait first for submission, before waiting for
> request completion")
> Fixes: 221fe7994554 ("drm/i915: Perform a direct reset of the GPU from the
> waiter")
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 44 +---
> drivers/gpu/drm/i915/intel_display.c | 199
> ---
> 3 files changed, 139 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index effbe4f72a64..88ddd27f61b0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gp
Re: [PATCH] drm/vblank: Fix race between drm_crtc_arm_vblank_event() and the irq
On Fri, Jun 30, 2017 at 03:12:51PM +0200, Daniel Vetter wrote: > On Fri, Jun 30, 2017 at 03:27:29PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Make sure the armed even doesn't fire until the next vblank by adding > > one to the current vblank counter value. This will prevent the event > > being fired off prematurely if drm_handle_vblank() is called > > redundantly, or if the irq handler gets delayed somehow. > > > > This is actually a real race on Intel gen2/3 hardware due to the vblank > > interrupt firing approx. one or more scanlines after the start of vblank > > (which is when the double buffered registers get latched). So if we were > > to perform an atomic update just after the start of vblank, but before > > the irq has fired, the irq handler would send out the the event immediately > > instead of waiting for the next vblank like it should. > > > > This also makes logs less confusing because now it normally looks like > > the vblank interrupt was somehow missed and the event gets sent one > > frame too late. > > So this para here I like, since the others are already written off in the > kernel-doc as "don't use this function if this is a problem for you". I'd > say your claim is even wrong, since we're not using > drm_accurate_vblank_count, and so the race still exists (if you're > sufficiently unlucky at least, and since there can be an arbitrary delay > between when the hw raises an irq and when the kernel runs the irq > handler it's not any better). At least for i915 it should be accurate if the irq wasn't enabled prior to starting the pipe update as we'll update the counter when enabling the irq. To make sure it's accurate we should probably have a drm_vblank_get_accurate() or something like that. > > Anyway, as long as it's not cc: stable (that would need more convincing on > my side): > > Reviewed-by: Daniel Vetter > > > > Cc: Daniel Vetter > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/drm_vblank.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > index bfe86fa2d3b1..823c853de052 100644 > > --- a/drivers/gpu/drm/drm_vblank.c > > +++ b/drivers/gpu/drm/drm_vblank.c > > @@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, > > assert_spin_locked(&dev->event_lock); > > > > e->pipe = pipe; > > - e->event.sequence = drm_vblank_count(dev, pipe); > > + e->event.sequence = drm_vblank_count(dev, pipe) + 1; > > e->event.crtc_id = crtc->base.id; > > list_add_tail(&e->base.link, &dev->vblank_event_list); > > } > > -- > > 2.13.0 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RESEND-CI v4 15/15] drm/i915/glk: set HDMI 2.0 identifier
On Fri, 2017-06-30 at 17:47 +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 6/30/2017 5:37 PM, Ander Conselvan De Oliveira wrote:
> > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > This patch sets the is_hdmi2_src identifier in drm connector
> > > for GLK platform. GLK contains a native HDMI 2.0 controller.
> > > This identifier will help the EDID handling functions to save
> > > lot of work which is specific to HDMI 2.0 sources.
> > >
> > > V3: Added this patch
> > > V4: Rebase
> > >
> > > Signed-off-by: Shashank Sharma
> >
> > This and patches 12 and 14 look fine to me. I'm not sure about the patch
> > split,
> > maybe they should be squashed together in the end? And perhaps patch 10 and
> > 13
> > too if the refactor I proposed are separate prep patches.
>
> In fact this is exactly how I prepared at the first place, keeping all
> the crtc/pipe level changes together.
> But then I thought the patch might be touching too many things, and
> might be too big or complex for
> review, that's why I had split into 3-4 small patches :-)
> > But anyway, you can
> > use
> >
> > Reviewed-by: Ander Conselvan de Oliveira
> >
> > on those if you want.
>
> Thanks, I guess this applies for 12,14,10,13 and 15 (with a separate
> patch for CSC coeff handling).
> Please correct me if I misunderstood.
This applies to 12, 14 and 15.
Ander
>
> - Shashank
> >
> >
> > > ---
> > > drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > > b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index 3bd9af3..0d9d088 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1979,6 +1979,9 @@ void intel_hdmi_init_connector(struct
> > > intel_digital_port *intel_dig_port,
> > > connector->doublescan_allowed = 0;
> > > connector->stereo_allowed = 1;
> > >
> > > + if (IS_GEMINILAKE(dev_priv))
> > > + connector->ycbcr_420_allowed = true;
> > > +
> > > intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > >
> > > switch (port) {
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 03:25:58PM +0200, Daniel Vetter wrote: > On Thu, Jun 29, 2017 at 04:49:48PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > Introduce an rw_semaphore to protect the display commits. All normal > > commits use down_read() and hence can proceed in parallel, but GPU reset > > will use down_write() making sure no other commits are in progress when > > we have to pull the plug on the display engine on pre-g4x platforms. > > There are no modeset/gem locks taken inside __intel_atomic_commit_tail() > > itself, and we wait for all dependencies before the down_read(), and > > thus there is no chance of deadlocks with this scheme. > > How does this solve the deadlock? Afaiui the deadlock is that the gpu > reset stopped unconditionally completing all requests before it did > anything else, including anything with the hw or taking modeset locks. > > This ensured that any outstanding flips (we only had pageflips, no atomic > ones back then) could complete (although maybe displaying garbage). The > only thing we had to do was grab the locks (to avoid concurrent modesets) > and then we could happily nuke the hw (since the flips where lost in the > CS anyway), and restore it afterwards. > > Then the TDR rewrite came around and broke that, which now makes atomic > time out waiting for the gpu to complete (because the gpu reset waits for > the modeset to complete first). Which means if you want to fix this > without breaking TDR, then you need to cancel the pending atomic commits. > That seems somewhat what you're attempting here with trying to figure out > what the latest hw-committed step is (but that function gets it wrong), > and lots more locking and stuff on top. > > Why exactly can't we just go back to the old world of force-completing > dead requests on gen4 and earlier? That would be tons simpler imo instead > of throwing a pile of hacks (which really needs a complete rewrite of the > atomic code in i915) in as a work-around. We didn't have TDR on gen4 and > earlier for years, going back to that isn't going to hurt anyone. > > Making working gen4 gpu reset contigent on cancellable atomic commits and > the full commit queue is imo nuts. And if the GEM folks insist the old behavior can't be restored, then we just need a tailor-made get-out-of-jail card for gen4 gpu reset somewhere in i915_sw_fence. Force-completing all render requests atomic updates depend upon is imo the simplest solution to this, and we've had a driver that worked like that for years. And as long as TDR resubmits the batches the render-glitch will at most be temporary, but that's totally fine: We're killing the entire display block in the reset anyway, there's no way the user won't notice _that_ kind of glitch. Either way, let's please not over-engineer something for a dead-old platform when something much, much, much simpler worked for years, and should keep on working for another few years. -Daniel PS: One issue with atomic is that there's the small matter of having to wait for pending atomic commits to complete before we nuke the display, to avoid upsetting the display code. We could do that with a dummy commit, or just having a special wait_for_depencies that waits for all CRTC to complete their pending atomic updates. -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Thu, Jun 29, 2017 at 10:26:08PM +0300, Ville Syrjälä wrote: > On Thu, Jun 29, 2017 at 06:57:30PM +0100, Chris Wilson wrote: > > Quoting ville.syrj...@linux.intel.com (2017-06-29 15:36:42) > > > From: Ville Syrjälä > > > > > > Introduce an rw_semaphore to protect the display commits. All normal > > > commits use down_read() and hence can proceed in parallel, but GPU reset > > > will use down_write() making sure no other commits are in progress when > > > we have to pull the plug on the display engine on pre-g4x platforms. > > > There are no modeset/gem locks taken inside __intel_atomic_commit_tail() > > > itself, and we wait for all dependencies before the down_read(), and > > > thus there is no chance of deadlocks with this scheme. > > > > This matches what I thought should be done (I didn't think of using > > rwsem just a mutex, nice touch). The point I got stuck on was what > > should be done after the reset? I expected another modeset to return the > > state back or otherwise the inflight would get confused? > > I guess that can happen. For instance, if we have a crtc_enable() in flight, > and then we do a reset before it gets committed we would end up doing > crtc_enable() twice in a row without a crtc_disable in between. For page > flips and such this shouldn't be a big deal in general. atomic commits are ordered. You have to wait for the previous ones to complete before you do a new one. If you don't do that, then all hell breaks loose. What you really can't do with atomic (without rewriting everything once more) is cancel a commit. Pre-atomic we could do that on gen4 since the mmio flips died with the gpu, but that's the one design change we need to cope with (plus TDR insisting it can't force-complete requests anymore). > > > During reset we should be recommiting the state that was committed last. > > > But for now we'll settle for recommiting the last state for each object. > > > > Ah, I guess that explains the above. What is the complication with > > restoring the current state as opposed to the next state? > > Well the main thing is just tracking which is the current state. That > just needs refactoring the .atomic_duplicate_state() calling convention > across the whole tree so that we can then duplicate the committed state > rather than the latest state. > > Also due to the commit_hw_done() being potentially done after the > modeset locks have been dropped, I don't think we can be certain > of it getting called in the same order as swap_state(), hence > when we track the committed state in commit_hw_done() we'll have > to have some way to figure out if our new state is in fact the > latest committed state for each object or if the calls got > reordered. We don't insert any dependencies between two commits > unless they touch the same active crtc, thus this reordering > seems very much possible. Dunno if we should add some way to add > such dependeencies whenever the same object is part of two otherwise > independent commits, or if we just want to try and work with the > reordered calls. My idea for the latter was some kind of seqno/age > counter on the object states that allows me to recongnize which > state is more recent. The object states aren't refcounted so hanging > on to the wrong pointer could cause an oops the next time we have to > perform a GPU reset. Atomic commits are strongly ordered on a given CRTC, so stuff can't be out-of-order within one. Across them the idea was to just add more CRTC states into the atomic commit to make sure stuff is ordered correctly. Laurent just pointed out that for switching planes that doesn't happen atm, but for i915 we should be safe (I guess I thought too much about i915 when typing the commit tracking code). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vblank: Fix race between drm_crtc_arm_vblank_event() and the irq
On Fri, Jun 30, 2017 at 04:26:49PM +0300, Ville Syrjälä wrote: > On Fri, Jun 30, 2017 at 03:12:51PM +0200, Daniel Vetter wrote: > > On Fri, Jun 30, 2017 at 03:27:29PM +0300, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > Make sure the armed even doesn't fire until the next vblank by adding > > > one to the current vblank counter value. This will prevent the event > > > being fired off prematurely if drm_handle_vblank() is called > > > redundantly, or if the irq handler gets delayed somehow. > > > > > > This is actually a real race on Intel gen2/3 hardware due to the vblank > > > interrupt firing approx. one or more scanlines after the start of vblank > > > (which is when the double buffered registers get latched). So if we were > > > to perform an atomic update just after the start of vblank, but before > > > the irq has fired, the irq handler would send out the the event > > > immediately > > > instead of waiting for the next vblank like it should. > > > > > > This also makes logs less confusing because now it normally looks like > > > the vblank interrupt was somehow missed and the event gets sent one > > > frame too late. > > > > So this para here I like, since the others are already written off in the > > kernel-doc as "don't use this function if this is a problem for you". I'd > > say your claim is even wrong, since we're not using > > drm_accurate_vblank_count, and so the race still exists (if you're > > sufficiently unlucky at least, and since there can be an arbitrary delay > > between when the hw raises an irq and when the kernel runs the irq > > handler it's not any better). > > At least for i915 it should be accurate if the irq wasn't enabled > prior to starting the pipe update as we'll update the counter when > enabling the irq. To make sure it's accurate we should probably > have a drm_vblank_get_accurate() or something like that. We could switch the drm_vblank_count in the diff below to drm_accurate_vblank_count. I typed this helper for drivers which don't have accurate vblank timestamping support, so the difference was moot. But with i915 using it that's changed (and iirc vc4 uses it too or something like that). -Daniel > > > > > Anyway, as long as it's not cc: stable (that would need more convincing on > > my side): > > > > Reviewed-by: Daniel Vetter > > > > > > Cc: Daniel Vetter > > > Signed-off-by: Ville Syrjälä > > > --- > > > drivers/gpu/drm/drm_vblank.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > > index bfe86fa2d3b1..823c853de052 100644 > > > --- a/drivers/gpu/drm/drm_vblank.c > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > @@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc, > > > assert_spin_locked(&dev->event_lock); > > > > > > e->pipe = pipe; > > > - e->event.sequence = drm_vblank_count(dev, pipe); > > > + e->event.sequence = drm_vblank_count(dev, pipe) + 1; > > > e->event.crtc_id = crtc->base.id; > > > list_add_tail(&e->base.link, &dev->vblank_event_list); > > > } > > > -- > > > 2.13.0 > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > -- > Ville Syrjälä > Intel OTC -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm: vmwgfx: Remove the legacy CRTC .prepare() helper operations
Reviewed-by: Thomas Hellstrom
On 06/30/2017 11:36 AM, Laurent Pinchart wrote:
The CRTC .prepare() helper operation is legacy code, drivers should
use the .atomic_disable() operation instead.
When a CRTC is temporarily disabled prior to a mode set, the atomic
helpers call the .prepare() operation if provided instead of the
.atomic_disable() operation. In order to avoid modifying the driver's
behaviour that has an empty .prepare() implementation, we need to return
from the .atomic_disable() operation without performing any action if
the CRTC will be reenabled.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 15 +++
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 +++---
2 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 854403509216..bdf6349de250 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -259,17 +259,6 @@ static void vmw_sou_crtc_mode_set_nofb(struct drm_crtc
*crtc)
}
/**
- * vmw_sou_crtc_helper_prepare - Noop
- *
- * @crtc: CRTC associated with the new screen
- *
- * Prepares the CRTC for a mode set, but we don't need to do anything here.
- */
-static void vmw_sou_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-/**
* vmw_sou_crtc_atomic_enable - Noop
*
* @crtc: CRTC associated with the new screen
@@ -299,6 +288,9 @@ static void vmw_sou_crtc_atomic_disable(struct drm_crtc
*crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
sou = vmw_crtc_to_sou(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -574,7 +566,6 @@ drm_plane_helper_funcs vmw_sou_primary_plane_helper_funcs = {
};
static const struct drm_crtc_helper_funcs vmw_sou_crtc_helper_funcs = {
- .prepare = vmw_sou_crtc_helper_prepare,
.mode_set_nofb = vmw_sou_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ed9404a7f457..c3bd4a012b64 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -406,12 +406,6 @@ static void vmw_stdu_crtc_mode_set_nofb(struct drm_crtc
*crtc)
crtc->x, crtc->y);
}
-
-static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-
static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
{
@@ -446,6 +440,9 @@ static void vmw_stdu_crtc_atomic_disable(struct drm_crtc
*crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
stdu = vmw_crtc_to_stdu(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -1416,7 +1413,6 @@ drm_plane_helper_funcs vmw_stdu_primary_plane_helper_funcs = {
};
static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs = {
- .prepare = vmw_stdu_crtc_helper_prepare,
.mode_set_nofb = vmw_stdu_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm: vmwgfx: Remove the legacy CRTC .prepare() helper operations
Want me to throw this into drm-misc too, or will you pick this up in
wmvgfx trees? Since it's not tree-wide there's no reason for it to go
through drm-misc.
-Daniel
On Fri, Jun 30, 2017 at 3:04 PM, Thomas Hellstrom wrote:
> Reviewed-by: Thomas Hellstrom
>
>
>
> On 06/30/2017 11:36 AM, Laurent Pinchart wrote:
>>
>> The CRTC .prepare() helper operation is legacy code, drivers should
>> use the .atomic_disable() operation instead.
>>
>> When a CRTC is temporarily disabled prior to a mode set, the atomic
>> helpers call the .prepare() operation if provided instead of the
>> .atomic_disable() operation. In order to avoid modifying the driver's
>> behaviour that has an empty .prepare() implementation, we need to return
>> from the .atomic_disable() operation without performing any action if
>> the CRTC will be reenabled.
>>
>> Signed-off-by: Laurent Pinchart
>>
>> ---
>> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 15 +++
>> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 +++---
>> 2 files changed, 6 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>> index 854403509216..bdf6349de250 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
>> @@ -259,17 +259,6 @@ static void vmw_sou_crtc_mode_set_nofb(struct
>> drm_crtc *crtc)
>> }
>> /**
>> - * vmw_sou_crtc_helper_prepare - Noop
>> - *
>> - * @crtc: CRTC associated with the new screen
>> - *
>> - * Prepares the CRTC for a mode set, but we don't need to do anything
>> here.
>> - */
>> -static void vmw_sou_crtc_helper_prepare(struct drm_crtc *crtc)
>> -{
>> -}
>> -
>> -/**
>>* vmw_sou_crtc_atomic_enable - Noop
>>*
>>* @crtc: CRTC associated with the new screen
>> @@ -299,6 +288,9 @@ static void vmw_sou_crtc_atomic_disable(struct
>> drm_crtc *crtc,
>> return;
>> }
>> + if (crtc->state->enable)
>> + return;
>> +
>> sou = vmw_crtc_to_sou(crtc);
>> dev_priv = vmw_priv(crtc->dev);
>> @@ -574,7 +566,6 @@ drm_plane_helper_funcs
>> vmw_sou_primary_plane_helper_funcs = {
>> };
>> static const struct drm_crtc_helper_funcs vmw_sou_crtc_helper_funcs =
>> {
>> - .prepare = vmw_sou_crtc_helper_prepare,
>> .mode_set_nofb = vmw_sou_crtc_mode_set_nofb,
>> .atomic_check = vmw_du_crtc_atomic_check,
>> .atomic_begin = vmw_du_crtc_atomic_begin,
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>> b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>> index ed9404a7f457..c3bd4a012b64 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
>> @@ -406,12 +406,6 @@ static void vmw_stdu_crtc_mode_set_nofb(struct
>> drm_crtc *crtc)
>> crtc->x, crtc->y);
>> }
>> -
>> -static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
>> -{
>> -}
>> -
>> -
>> static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
>> struct drm_crtc_state *old_state)
>> {
>> @@ -446,6 +440,9 @@ static void vmw_stdu_crtc_atomic_disable(struct
>> drm_crtc *crtc,
>> return;
>> }
>> + if (crtc->state->enable)
>> + return;
>> +
>> stdu = vmw_crtc_to_stdu(crtc);
>> dev_priv = vmw_priv(crtc->dev);
>> @@ -1416,7 +1413,6 @@ drm_plane_helper_funcs
>> vmw_stdu_primary_plane_helper_funcs = {
>> };
>> static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs =
>> {
>> - .prepare = vmw_stdu_crtc_helper_prepare,
>> .mode_set_nofb = vmw_stdu_crtc_mode_set_nofb,
>> .atomic_check = vmw_du_crtc_atomic_check,
>> .atomic_begin = vmw_du_crtc_atomic_begin,
>
>
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/atomic: Wait indefinitely and interruptibly for hw_done.
On Wed, Jun 28, 2017 at 03:28:12PM +0200, Maarten Lankhorst wrote:
> Without waiting for hw_done, previous atomic updates may dereference
> the wrong state and cause a lot of confusion. The real fix is fixing
> all obj->state to use the accessor macros, but for now wait
> indefinitely and interruptibly.
>
> Cc: Boris Brezillon
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Cc: Sean Paul
> Cc: CK Hu
> Cc: Philipp Zabel
> Cc: Matthias Brugger
> Cc: Rob Clark
> Cc: Ben Skeggs
> Cc: Thierry Reding
> Cc: Jonathan Hunter
> Cc: Jyri Sarha
> Cc: Tomi Valkeinen
> Cc: Eric Anholt
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-te...@vger.kernel.org
> Signed-off-by: Maarten Lankhorst
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 12 +---
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f66b6c45cdd0..56e7729d993d 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2144,8 +2144,7 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> int drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> bool stall)
Needs improved kernel-doc:
* Returns:
*
* 0 on success. Can return -EINTR when @stall is true and the waiting for
* the previous commits has been interrupted.
With that Reviewed-by: Daniel Vetter
> {
> - int i;
> - long ret;
> + int i, ret;
> struct drm_connector *connector;
> struct drm_connector_state *old_conn_state, *new_conn_state;
> struct drm_crtc *crtc;
> @@ -2168,12 +2167,11 @@ int drm_atomic_helper_swap_state(struct
> drm_atomic_state *state,
> if (!commit)
> continue;
>
> - ret = wait_for_completion_timeout(&commit->hw_done,
> - 10*HZ);
> - if (ret == 0)
> - DRM_ERROR("[CRTC:%d:%s] hw_done timed out\n",
> - crtc->base.id, crtc->name);
> + ret =
> wait_for_completion_interruptible(&commit->hw_done);
> drm_crtc_commit_put(commit);
> +
> + if (ret)
> + return ret;
> }
> }
>
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 03:35:03PM +0200, Daniel Vetter wrote: > On Thu, Jun 29, 2017 at 10:26:08PM +0300, Ville Syrjälä wrote: > > On Thu, Jun 29, 2017 at 06:57:30PM +0100, Chris Wilson wrote: > > > Quoting ville.syrj...@linux.intel.com (2017-06-29 15:36:42) > > > > From: Ville Syrjälä > > > > > > > > Introduce an rw_semaphore to protect the display commits. All normal > > > > commits use down_read() and hence can proceed in parallel, but GPU reset > > > > will use down_write() making sure no other commits are in progress when > > > > we have to pull the plug on the display engine on pre-g4x platforms. > > > > There are no modeset/gem locks taken inside __intel_atomic_commit_tail() > > > > itself, and we wait for all dependencies before the down_read(), and > > > > thus there is no chance of deadlocks with this scheme. > > > > > > This matches what I thought should be done (I didn't think of using > > > rwsem just a mutex, nice touch). The point I got stuck on was what > > > should be done after the reset? I expected another modeset to return the > > > state back or otherwise the inflight would get confused? > > > > I guess that can happen. For instance, if we have a crtc_enable() in flight, > > and then we do a reset before it gets committed we would end up doing > > crtc_enable() twice in a row without a crtc_disable in between. For page > > flips and such this shouldn't be a big deal in general. > > atomic commits are ordered. You have to wait for the previous ones to > complete before you do a new one. If you don't do that, then all hell > breaks loose. What we're effectively doing here is inserting two new commits in the middle of the timeline, one to disable everything, and another one to re-enable everything. At the end of the the re-enable we would want the hardware state should match exactly what was there before the disable, hence any commits still in the timeline should work correctly. That is, their old_state will match the hardware state when it comes time to commit them. But we can only do that properly after we start to track the committed state. Without that tracking we can get into trouble wrt. the hardware state not matching the old state when it comes time to commit the new state. > > What you really can't do with atomic (without rewriting everything once > more) is cancel a commit. Pre-atomic we could do that on gen4 since the > mmio flips died with the gpu, but that's the one design change we need to > cope with (plus TDR insisting it can't force-complete requests anymore). > > > > > During reset we should be recommiting the state that was committed last. > > > > But for now we'll settle for recommiting the last state for each object. > > > > > > Ah, I guess that explains the above. What is the complication with > > > restoring the current state as opposed to the next state? > > > > Well the main thing is just tracking which is the current state. That > > just needs refactoring the .atomic_duplicate_state() calling convention > > across the whole tree so that we can then duplicate the committed state > > rather than the latest state. > > > > Also due to the commit_hw_done() being potentially done after the > > modeset locks have been dropped, I don't think we can be certain > > of it getting called in the same order as swap_state(), hence > > when we track the committed state in commit_hw_done() we'll have > > to have some way to figure out if our new state is in fact the > > latest committed state for each object or if the calls got > > reordered. We don't insert any dependencies between two commits > > unless they touch the same active crtc, thus this reordering > > seems very much possible. Dunno if we should add some way to add > > such dependeencies whenever the same object is part of two otherwise > > independent commits, or if we just want to try and work with the > > reordered calls. My idea for the latter was some kind of seqno/age > > counter on the object states that allows me to recongnize which > > state is more recent. The object states aren't refcounted so hanging > > on to the wrong pointer could cause an oops the next time we have to > > perform a GPU reset. > > Atomic commits are strongly ordered on a given CRTC, so stuff can't be > out-of-order within one. Across them the idea was to just add more CRTC > states into the atomic commit to make sure stuff is ordered correctly. And atomic commits not touching the same crtc will not be ordered in any way. Thus if they touch the same object (eg. disabled plane or connector) we can't currently tell if the commit_hw_done() calls happened in the same order as the swap_state() calls for that particular object. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vblank: Fix race between drm_crtc_arm_vblank_event() and the irq
On Fri, Jun 30, 2017 at 03:36:50PM +0200, Daniel Vetter wrote: > On Fri, Jun 30, 2017 at 04:26:49PM +0300, Ville Syrjälä wrote: > > On Fri, Jun 30, 2017 at 03:12:51PM +0200, Daniel Vetter wrote: > > > On Fri, Jun 30, 2017 at 03:27:29PM +0300, ville.syrj...@linux.intel.com > > > wrote: > > > > From: Ville Syrjälä > > > > > > > > Make sure the armed even doesn't fire until the next vblank by adding > > > > one to the current vblank counter value. This will prevent the event > > > > being fired off prematurely if drm_handle_vblank() is called > > > > redundantly, or if the irq handler gets delayed somehow. > > > > > > > > This is actually a real race on Intel gen2/3 hardware due to the vblank > > > > interrupt firing approx. one or more scanlines after the start of vblank > > > > (which is when the double buffered registers get latched). So if we were > > > > to perform an atomic update just after the start of vblank, but before > > > > the irq has fired, the irq handler would send out the the event > > > > immediately > > > > instead of waiting for the next vblank like it should. > > > > > > > > This also makes logs less confusing because now it normally looks like > > > > the vblank interrupt was somehow missed and the event gets sent one > > > > frame too late. > > > > > > So this para here I like, since the others are already written off in the > > > kernel-doc as "don't use this function if this is a problem for you". I'd > > > say your claim is even wrong, since we're not using > > > drm_accurate_vblank_count, and so the race still exists (if you're > > > sufficiently unlucky at least, and since there can be an arbitrary delay > > > between when the hw raises an irq and when the kernel runs the irq > > > handler it's not any better). > > > > At least for i915 it should be accurate if the irq wasn't enabled > > prior to starting the pipe update as we'll update the counter when > > enabling the irq. To make sure it's accurate we should probably > > have a drm_vblank_get_accurate() or something like that. > > We could switch the drm_vblank_count in the diff below to > drm_accurate_vblank_count. That's somewhat sub-optimal as we can avoid the update when we know that drm_vblank_get() already did it for us. > I typed this helper for drivers which don't > have accurate vblank timestamping support, so the difference was moot. But > with i915 using it that's changed (and iirc vc4 uses it too or something > like that). > -Daniel > > > > > > > > > Anyway, as long as it's not cc: stable (that would need more convincing on > > > my side): > > > > > > Reviewed-by: Daniel Vetter > > > > > > > > Cc: Daniel Vetter > > > > Signed-off-by: Ville Syrjälä > > > > --- > > > > drivers/gpu/drm/drm_vblank.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > > > > index bfe86fa2d3b1..823c853de052 100644 > > > > --- a/drivers/gpu/drm/drm_vblank.c > > > > +++ b/drivers/gpu/drm/drm_vblank.c > > > > @@ -869,7 +869,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc > > > > *crtc, > > > > assert_spin_locked(&dev->event_lock); > > > > > > > > e->pipe = pipe; > > > > - e->event.sequence = drm_vblank_count(dev, pipe); > > > > + e->event.sequence = drm_vblank_count(dev, pipe) + 1; > > > > e->event.crtc_id = crtc->base.id; > > > > list_add_tail(&e->base.link, &dev->vblank_event_list); > > > > } > > > > -- > > > > 2.13.0 > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > > Ville Syrjälä > > Intel OTC > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/atomic: Change drm_atomic_helper_swap_state to return an error.
On Wed, Jun 28, 2017 at 03:28:11PM +0200, Maarten Lankhorst wrote:
> We want to change swap_state to wait indefinitely, but to do this
> swap_state should wait interruptibly. This requires propagating
> the error to each driver. All drivers have changes to deal with the
> clean up. In order to allow easy reverting, the commit that changes
> behavior is separate so someone only has to revert that for testing.
>
> Nouveau has a small bugfix, if drm_atomic_helper_wait_for_fences
> failed cleanup_planes was not called.
>
> Cc: Boris Brezillon
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Jani Nikula
> Cc: Sean Paul
> Cc: CK Hu
> Cc: Philipp Zabel
> Cc: Matthias Brugger
> Cc: Rob Clark
> Cc: Ben Skeggs
> Cc: Thierry Reding
> Cc: Jonathan Hunter
> Cc: Jyri Sarha
> Cc: Tomi Valkeinen
> Cc: Eric Anholt
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-ker...@vger.kernel.org
> Cc: intel-...@lists.freedesktop.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: freedr...@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: linux-te...@vger.kernel.org
> Signed-off-by: Maarten Lankhorst
We kinda need to backport this to older kernels, but I don't really see
how :( Maybe we should split this up:
patch 1: Change to int return type
patches 2-(n-1): Driver conversions
patch n: __must_check addition
That would at least somewhat make this backportable ...
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 14 --
> drivers/gpu/drm/drm_atomic_helper.c | 18 --
> drivers/gpu/drm/i915/intel_display.c | 10 +-
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 ++-
> drivers/gpu/drm/msm/msm_atomic.c | 14 +-
> drivers/gpu/drm/nouveau/nv50_display.c | 10 --
> drivers/gpu/drm/tegra/drm.c | 7 ++-
> drivers/gpu/drm/tilcdc/tilcdc_drv.c | 6 +-
> drivers/gpu/drm/vc4/vc4_kms.c| 21 +
> include/drm/drm_atomic_helper.h | 4 ++--
> 10 files changed, 82 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 516d9547d331..d4f787bf1d4a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -544,9 +544,11 @@ static int atmel_hlcdc_dc_atomic_commit(struct
> drm_device *dev,
> goto error;
> }
>
> - /* Swap the state, this is the point of no return. */
> - drm_atomic_helper_swap_state(state, true);
Push the swap_state up over the commit setup (but after the allocation)
and there's no more a problem with unrolling.
> + ret = drm_atomic_helper_swap_state(state, true);
> + if (ret)
> + goto err_pending;
>
> + /* Swap state succeeded, this is the point of no return. */
> drm_atomic_state_get(state);
> if (async)
> queue_work(dc->wq, &commit->work);
> @@ -555,6 +557,14 @@ static int atmel_hlcdc_dc_atomic_commit(struct
> drm_device *dev,
>
> return 0;
>
> +err_pending:
> + /* Fail the commit, wake up any waiter. */
> + spin_lock(&dc->commit.wait.lock);
> + dc->commit.pending = false;
> + wake_up_all_locked(&dc->commit.wait);
> + spin_unlock(&dc->commit.wait.lock);
> +err_free:
> + kfree(commit);
> error:
> drm_atomic_helper_cleanup_planes(dev, state);
> return ret;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index f1847d29ba3e..f66b6c45cdd0 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1387,10 +1387,8 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>
> if (!nonblock) {
> ret = drm_atomic_helper_wait_for_fences(dev, state, true);
> - if (ret) {
> - drm_atomic_helper_cleanup_planes(dev, state);
> - return ret;
> - }
> + if (ret)
> + goto err;
> }
>
> /*
> @@ -1399,7 +1397,9 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>* the software side now.
>*/
>
> - drm_atomic_helper_swap_state(state, true);
> + ret = drm_atomic_helper_swap_state(state, true);
> + if (ret)
> + goto err;
>
> /*
>* Everything below can be run asynchronously without the need to grab
> @@ -1428,6 +1428,10 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> commit_tail(state);
>
> return 0;
> +
> +err:
> + drm_atomic_helper_cleanup_planes(dev, state);
> + return ret;
> }
> EXPORT_SYMBOL(drm_atomic_helper_commit);
>
> @@ -2137,7 +2141,7 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> * the current atomic helpers this is almost always the case, since the
Re: [PATCH v2 3/3] drm: vmwgfx: Remove the legacy CRTC .prepare() helper operations
Hi, Daniel,
If possible, throw it into drm-misc. We have no changes for the next
merge window at this time so it's a bit unnecessary to generate a
separate pull request for this.
Thanks,
Thomas
On 06/30/2017 03:39 PM, Daniel Vetter wrote:
Want me to throw this into drm-misc too, or will you pick this up in
wmvgfx trees? Since it's not tree-wide there's no reason for it to go
through drm-misc.
-Daniel
On Fri, Jun 30, 2017 at 3:04 PM, Thomas Hellstrom wrote:
Reviewed-by: Thomas Hellstrom
On 06/30/2017 11:36 AM, Laurent Pinchart wrote:
The CRTC .prepare() helper operation is legacy code, drivers should
use the .atomic_disable() operation instead.
When a CRTC is temporarily disabled prior to a mode set, the atomic
helpers call the .prepare() operation if provided instead of the
.atomic_disable() operation. In order to avoid modifying the driver's
behaviour that has an empty .prepare() implementation, we need to return
from the .atomic_disable() operation without performing any action if
the CRTC will be reenabled.
Signed-off-by: Laurent Pinchart
---
drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 15 +++
drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 10 +++---
2 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 854403509216..bdf6349de250 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -259,17 +259,6 @@ static void vmw_sou_crtc_mode_set_nofb(struct
drm_crtc *crtc)
}
/**
- * vmw_sou_crtc_helper_prepare - Noop
- *
- * @crtc: CRTC associated with the new screen
- *
- * Prepares the CRTC for a mode set, but we don't need to do anything
here.
- */
-static void vmw_sou_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-/**
* vmw_sou_crtc_atomic_enable - Noop
*
* @crtc: CRTC associated with the new screen
@@ -299,6 +288,9 @@ static void vmw_sou_crtc_atomic_disable(struct
drm_crtc *crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
sou = vmw_crtc_to_sou(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -574,7 +566,6 @@ drm_plane_helper_funcs
vmw_sou_primary_plane_helper_funcs = {
};
static const struct drm_crtc_helper_funcs vmw_sou_crtc_helper_funcs =
{
- .prepare = vmw_sou_crtc_helper_prepare,
.mode_set_nofb = vmw_sou_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index ed9404a7f457..c3bd4a012b64 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -406,12 +406,6 @@ static void vmw_stdu_crtc_mode_set_nofb(struct
drm_crtc *crtc)
crtc->x, crtc->y);
}
-
-static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
-{
-}
-
-
static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_state)
{
@@ -446,6 +440,9 @@ static void vmw_stdu_crtc_atomic_disable(struct
drm_crtc *crtc,
return;
}
+ if (crtc->state->enable)
+ return;
+
stdu = vmw_crtc_to_stdu(crtc);
dev_priv = vmw_priv(crtc->dev);
@@ -1416,7 +1413,6 @@ drm_plane_helper_funcs
vmw_stdu_primary_plane_helper_funcs = {
};
static const struct drm_crtc_helper_funcs vmw_stdu_crtc_helper_funcs =
{
- .prepare = vmw_stdu_crtc_helper_prepare,
.mode_set_nofb = vmw_stdu_crtc_mode_set_nofb,
.atomic_check = vmw_du_crtc_atomic_check,
.atomic_begin = vmw_du_crtc_atomic_begin,
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIBaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=zvSnZOcctIomH3uHnUlUtGM3V2HQx8fRxI5e3gP1IWw&s=MsJ6mDVhjiu2b7IH0naLkx-srL98cgLZ5fXSilipnKI&e=
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101654] The Witcher 3: ground is too dark in some areas
https://bugs.freedesktop.org/show_bug.cgi?id=101654 Shmerl changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #1 from Shmerl --- After all, Nvidia blob is also affected by this, so it's probably a Wine bug. Closing for now. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101653] FAN speed is too high when displays are off
https://bugs.freedesktop.org/show_bug.cgi?id=101653 --- Comment #3 from Alex Deucher --- Created attachment 132374 --> https://bugs.freedesktop.org/attachment.cgi?id=132374&action=edit possible fix Depends on attachment 132358. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 196197] [drm:r600_ring_test [radeon]] *ERROR* radeon: ring 0 test failed (scratch(0x8504)=0xCAFEDEAD)
https://bugzilla.kernel.org/show_bug.cgi?id=196197 --- Comment #7 from Andreas Brogle (an...@ok.de) --- Hello Michel, is that what you need ? The last 7 turns caused a kernel panic, all the 4.10.0-rc1 versions. I declared them as bad with git bisect. Hence I can't say if that ring 0 bug does exist or not for those versions. root@a1:/work/linux-4.12-rc7# git bisect bad 0fc1223f0e77a748f7040562faaa7027f7db71ca is the first bad commit commit 0fc1223f0e77a748f7040562faaa7027f7db71ca Author: Rajat Jain Date: Mon Jan 2 22:34:10 2017 -0800 PCI/ASPM: Add L1 substate capability structure register definitions Add L1 substate capability structure register definitions for use in subsequent patches. See the PCIe r3.1 spec, sec 7.33. [bhelgaas: add PCIe spec reference] Signed-off-by: Rajat Jain Signed-off-by: Bjorn Helgaas :04 04 fd3e716ea9d6ed71cea541a3cc3a999e92c188ca 8a22f20457cd2bdd9f6f3fe8ad43ecba629a95bd M include root@a1:/work/linux-4.12-rc7# git bisect log git bisect start # bad: [c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201] Linux 4.11-rc1 git bisect bad c1ae3cfa0e89fa1a7ecc4c99031f5e9ae99d9201 # good: [7089db84e356562f8ba737c29e472cc42d530dbc] Linux 4.10-rc8 git bisect good 7089db84e356562f8ba737c29e472cc42d530dbc # good: [caa59428971d5ad81d19512365c9ba580d83268c] Merge tag 'staging-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging git bisect good caa59428971d5ad81d19512365c9ba580d83268c # bad: [ca2dea434d10e3a676482fdf6df17d20cdb3e907] Merge tag 'juno-fixes-4.11' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux into next/late git bisect bad ca2dea434d10e3a676482fdf6df17d20cdb3e907 # good: [28cbc335d272f293c4368abd4ac2e17e36805b79] Merge tag 'sound-4.11-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound git bisect good 28cbc335d272f293c4368abd4ac2e17e36805b79 # bad: [54fff785db6e44208478ae3b0e5c56b853b3e10d] Merge tag 'armsoc-defconfig' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc git bisect bad 54fff785db6e44208478ae3b0e5c56b853b3e10d # good: [190c3ee06a0f0660839785b7ad8a830e832d9481] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net git bisect good 190c3ee06a0f0660839785b7ad8a830e832d9481 # bad: [af8999f672421776417977101c3e1f334414c065] Merge tag 'armsoc-fixes-nc' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc git bisect bad af8999f672421776417977101c3e1f334414c065 # bad: [b2e6d3055d5545b97533d4e8376fa848639d9951] Merge branch 'pci/host-exynos' into next git bisect bad b2e6d3055d5545b97533d4e8376fa848639d9951 # bad: [0dc49eb2a25a6b48e235803eeb2b52677f577ca5] Merge branch 'pci/resource' into next git bisect bad 0dc49eb2a25a6b48e235803eeb2b52677f577ca5 # bad: [63ab93f021ecd815d848c3e9d3e326fa9628e5a9] Merge branch 'pci/enumeration' into next git bisect bad 63ab93f021ecd815d848c3e9d3e326fa9628e5a9 # bad: [8f417ca9ebfa8701a41db64f5ed9cbb01b8e4219] Merge branch 'pci/aspm' into next git bisect bad 8f417ca9ebfa8701a41db64f5ed9cbb01b8e4219 # bad: [f1f0366dd6be9624f7d355b72cc909ab821eb4c0] PCI/ASPM: Calculate and save the L1.2 timing parameters git bisect bad f1f0366dd6be9624f7d355b72cc909ab821eb4c0 # bad: [b2103ccbb67e3ef0f7a75d21c989f9614ddbcaca] PCI/ASPM: Add support for L1 substates git bisect bad b2103ccbb67e3ef0f7a75d21c989f9614ddbcaca # bad: [0fc1223f0e77a748f7040562faaa7027f7db71ca] PCI/ASPM: Add L1 substate capability structure register definitions git bisect bad 0fc1223f0e77a748f7040562faaa7027f7db71ca # first bad commit: [0fc1223f0e77a748f7040562faaa7027f7db71ca] PCI/ASPM: Add L1 substate capability structure register definitions Greetings, Andreas -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset
On Fri, 2017-06-30 at 17:29 +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 6/30/2017 5:04 PM, Ander Conselvan De Oliveira wrote:
> > On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
> > > Regards
> > >
> > > Shashank
> > >
> > >
> > > On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
> > > > On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
> > > > > To get a YCBCR420 output from intel platforms, we need one
> > > > > scaler to scale down YCBCR444 samples to YCBCR420 samples.
> > > > >
> > > > > This patch:
> > > > > - Does scaler allocation for HDMI ycbcr420 outputs.
> > > > > - Programs PIPE_MISC register for ycbcr420 output.
> > > > > - Adds a new scaler user "HDMI output" to plug-into existing
> > > > > scaler framework. This output type is identified using bit
> > > > > 30 of the scaler users bitmap.
> > > > >
> > > > > V2: rebase
> > > > > V3: rebase
> > > > > V4: rebase
> > > > >
> > > > > Cc: Ville Syrjala
> > > > > Cc: Ander Conselvan De Oliveira
> > > > >
> > > > > Signed-off-by: Shashank Sharma
> > > > > ---
> > > > >drivers/gpu/drm/i915/intel_atomic.c | 6 ++
> > > > >drivers/gpu/drm/i915/intel_display.c | 26
> > > > > ++
> > > > >drivers/gpu/drm/i915/intel_drv.h | 10 +-
> > > > >drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
> > > > >drivers/gpu/drm/i915/intel_panel.c | 3 ++-
> > > > >5 files changed, 53 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > > > > b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > index 36d4e63..a8c9ac5 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > > > > @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct
> > > > > drm_i915_private *dev_priv,
> > > > >
> > > > > /* panel fitter case: assign as a crtc scaler */
> > > > > scaler_id = &scaler_state->scaler_id;
> > > > > + } else if (i == SKL_HDMI_OUTPUT_INDEX) {
> > > > > + name = "HDMI-OUTPUT";
> > > > > + idx = intel_crtc->base.base.id;
> > > > > +
> > > > > + /* hdmi output case: needs a pipe scaler */
> > > > > + scaler_id = &scaler_state->scaler_id;
> > > > > } else {
> > > > > name = "PLANE";
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index da29536..983f581 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state
> > > > > *crtc_state, bool force_detach,
> > > > >*/
> > > > > need_scaling = src_w != dst_w || src_h != dst_h;
> > > > >
> > > > > + if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
> > > > > + && scaler_user == SKL_HDMI_OUTPUT_INDEX)
> > > >
> > > > Is it really necessary to check for both? If it is, what's the point of
> > > > creating
> > > > SKL_HDMI_OUTPUT_INDEX?
> > >
> > > Yes, else this will affect scaler update for planes, as this function
> > > gets called to update the plane scalers too, at that time the output
> > > will be still valid (as its at CRTC level), but the
> > > scaler user would be different.
> >
> > Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
> > wouldn't it be a bug if hdmi_output != YCBCR420 ? Point is, if the caller
> > asked
> > for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to
> > second
> > guess it.
>
> skl_update_scaler function gets called for all the users, which are:
> - panel fitter
> - all the planes
> - newly added user hdmi_output
> what about the case (assume) when we have handled hdmi_output and now we
> are handling for a plane, which doesnt need scaling.
>
> in this case, the code will look like:
> if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420)
>need_scaling = true /* which is wrong, as this function call is
> for plane scalar, and scaler_user = scaler */
>
> if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420 &&
> scaler_user == HDMI_OUT)
> need_scaling = true /* this is correct, as I want to consume scalar only
> when both the conditions are true.
I meant to do just
if (scaler_user == SKL_HDMI_OUTPUT_INDEX)
...;
I failed to notice the ambiguity in abbreviating to just HDMI_OUTPUT. But the
caller shouldn't request it if it doesn't need it, right?
>
> > > > > + /* YCBCR 444 -> 420 conversion needs a scaler */
> > > > > + need_scaling = true;
> > > > > +
> > > > > /*
> > > > >* if plane is being disabled or scaler is no more required or
> > > > > force detach
> > > > >* - free scaler binded to
[PATCH 3/3] drm/i915: Use drm_crtc_vblank_get_accurate() in intel_atomic_wait_for_vblanks()
From: Ville Syrjälä
To make sure intel_atomic_wait_for_vblanks() really waits for at least
one vblank let's switch to using drm_crtc_vblank_get_accurate().
Also toss in a FIXME that we should really be sampling the vblank
counter when we did the plane/wm update instead of resampling it
potentially much later when we call intel_atomic_wait_for_vblanks().
Cc: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4e03ca6c946f..12fc4fcf78c5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12826,12 +12826,16 @@ static void intel_atomic_wait_for_vblanks(struct
drm_device *dev,
if (!((1 << pipe) & crtc_mask))
continue;
- ret = drm_crtc_vblank_get(&crtc->base);
+ ret = drm_crtc_vblank_get_accurate(&crtc->base);
if (WARN_ON(ret != 0)) {
crtc_mask &= ~(1 << pipe);
continue;
}
+ /*
+* FIXME we should have sampled this
+* when we did the actual update.
+*/
last_vblank_count[pipe] = drm_crtc_vblank_count(&crtc->base);
}
--
2.13.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/3] drm/vblank: Introduce drm_crtc_vblank_get_accurate()
From: Ville Syrjälä
Add drm_crtc_vblank_get_accurate() which is the same as
drm_crtc_vblank_get() except that it will forcefully update the
the current vblank counter/timestamp value even if the interrupt
is already enabled.
And we'll switch drm_wait_one_vblank() over to using it to make sure it
will really wait at least one vblank even if it races with the irq
handler.
Cc: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_vblank.c | 37 -
include/drm/drm_vblank.h | 1 +
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 823c853de052..c57383b8753b 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -955,7 +955,8 @@ static int drm_vblank_enable(struct drm_device *dev,
unsigned int pipe)
return ret;
}
-static int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
+static int drm_vblank_get(struct drm_device *dev, unsigned int pipe,
+ bool force_update)
{
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
unsigned long irqflags;
@@ -975,6 +976,10 @@ static int drm_vblank_get(struct drm_device *dev, unsigned
int pipe)
if (!vblank->enabled) {
atomic_dec(&vblank->refcount);
ret = -EINVAL;
+ } else if (force_update) {
+ spin_lock(&dev->vblank_time_lock);
+ drm_update_vblank_count(dev, pipe, false);
+ spin_unlock(&dev->vblank_time_lock);
}
}
spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
@@ -994,10 +999,32 @@ static int drm_vblank_get(struct drm_device *dev,
unsigned int pipe)
*/
int drm_crtc_vblank_get(struct drm_crtc *crtc)
{
- return drm_vblank_get(crtc->dev, drm_crtc_index(crtc));
+ return drm_vblank_get(crtc->dev, drm_crtc_index(crtc), false);
}
EXPORT_SYMBOL(drm_crtc_vblank_get);
+/**
+ * drm_crtc_vblank_get_accurate - get a reference count on vblank events and
+ *make sure the counter is uptodate
+ * @crtc: which CRTC to own
+ *
+ * Acquire a reference count on vblank events to avoid having them disabled
+ * while in use.
+ *
+ * Also make sure the current vblank counter is value is fully up to date
+ * even if we're already past the start of vblank but the irq hasn't fired
+ * yet, which may be the case with some hardware that raises the interrupt
+ * only some time after the start of vblank.
+ *
+ * Returns:
+ * Zero on success or a negative error code on failure.
+ */
+int drm_crtc_vblank_get_accurate(struct drm_crtc *crtc)
+{
+ return drm_vblank_get(crtc->dev, drm_crtc_index(crtc), true);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_get_accurate);
+
static void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
{
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
@@ -1053,7 +1080,7 @@ void drm_wait_one_vblank(struct drm_device *dev, unsigned
int pipe)
if (WARN_ON(pipe >= dev->num_crtcs))
return;
- ret = drm_vblank_get(dev, pipe);
+ ret = drm_vblank_get(dev, pipe, true);
if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", pipe, ret))
return;
@@ -1248,7 +1275,7 @@ static void drm_legacy_vblank_pre_modeset(struct
drm_device *dev,
*/
if (!vblank->inmodeset) {
vblank->inmodeset = 0x1;
- if (drm_vblank_get(dev, pipe) == 0)
+ if (drm_vblank_get(dev, pipe, false) == 0)
vblank->inmodeset |= 0x2;
}
}
@@ -1448,7 +1475,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
*data,
return 0;
}
- ret = drm_vblank_get(dev, pipe);
+ ret = drm_vblank_get(dev, pipe, false);
if (ret) {
DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n",
pipe, ret);
return ret;
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 7fba9efe4951..5629e7841318 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -162,6 +162,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
int drm_crtc_vblank_get(struct drm_crtc *crtc);
+int drm_crtc_vblank_get_accurate(struct drm_crtc *crtc);
void drm_crtc_vblank_put(struct drm_crtc *crtc);
void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);
void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
--
2.13.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/i915: Fix race between pipe update and vblank irq
From: Ville Syrjälä
Make sure that we have an up to date vblank counter value for sending
out the completion event. On gen2/3 the vblank irq fires at frame start
rather than at start of vblank, so if we perform an update between
vblank start and frame start we would potentially sample a stale vblank
counter value, and thus send out the event one frame too soon.
Cc: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 0c650c2cbca8..61681a11086a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -197,7 +197,7 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct
intel_flip_work *work
* event outside of the critical section - the spinlock might spin for a
* while ... */
if (crtc->base.state->event) {
- WARN_ON(drm_crtc_vblank_get(&crtc->base) != 0);
+ WARN_ON(drm_crtc_vblank_get_accurate(&crtc->base) != 0);
spin_lock(&crtc->base.dev->event_lock);
drm_crtc_arm_vblank_event(&crtc->base, crtc->base.state->event);
--
2.13.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND-CI v4 11/15] drm/i915: prepare scaler for YCBCR420 modeset
Regards
Shashank
On 6/30/2017 7:45 PM, Ander Conselvan De Oliveira wrote:
On Fri, 2017-06-30 at 17:29 +0530, Sharma, Shashank wrote:
Regards
Shashank
On 6/30/2017 5:04 PM, Ander Conselvan De Oliveira wrote:
On Fri, 2017-06-30 at 11:20 +0530, Sharma, Shashank wrote:
Regards
Shashank
On 6/27/2017 5:46 PM, Ander Conselvan De Oliveira wrote:
On Wed, 2017-06-21 at 16:04 +0530, Shashank Sharma wrote:
To get a YCBCR420 output from intel platforms, we need one
scaler to scale down YCBCR444 samples to YCBCR420 samples.
This patch:
- Does scaler allocation for HDMI ycbcr420 outputs.
- Programs PIPE_MISC register for ycbcr420 output.
- Adds a new scaler user "HDMI output" to plug-into existing
scaler framework. This output type is identified using bit
30 of the scaler users bitmap.
V2: rebase
V3: rebase
V4: rebase
Cc: Ville Syrjala
Cc: Ander Conselvan De Oliveira
Signed-off-by: Shashank Sharma
---
drivers/gpu/drm/i915/intel_atomic.c | 6 ++
drivers/gpu/drm/i915/intel_display.c | 26 ++
drivers/gpu/drm/i915/intel_drv.h | 10 +-
drivers/gpu/drm/i915/intel_hdmi.c| 10 ++
drivers/gpu/drm/i915/intel_panel.c | 3 ++-
5 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index 36d4e63..a8c9ac5 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private
*dev_priv,
/* panel fitter case: assign as a crtc scaler */
scaler_id = &scaler_state->scaler_id;
+ } else if (i == SKL_HDMI_OUTPUT_INDEX) {
+ name = "HDMI-OUTPUT";
+ idx = intel_crtc->base.base.id;
+
+ /* hdmi output case: needs a pipe scaler */
+ scaler_id = &scaler_state->scaler_id;
} else {
name = "PLANE";
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index da29536..983f581 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
*/
need_scaling = src_w != dst_w || src_h != dst_h;
+ if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420
+ && scaler_user == SKL_HDMI_OUTPUT_INDEX)
Is it really necessary to check for both? If it is, what's the point of creating
SKL_HDMI_OUTPUT_INDEX?
Yes, else this will affect scaler update for planes, as this function
gets called to update the plane scalers too, at that time the output
will be still valid (as its at CRTC level), but the
scaler user would be different.
Right, so there is a need to check for scaler_user == HDMI_OUTPUT. But then
wouldn't it be a bug if hdmi_output != YCBCR420 ? Point is, if the caller asked
for a HDMI_OUTPUT scaler hopefully it knows what its doing, no need to second
guess it.
skl_update_scaler function gets called for all the users, which are:
- panel fitter
- all the planes
- newly added user hdmi_output
what about the case (assume) when we have handled hdmi_output and now we
are handling for a plane, which doesnt need scaling.
in this case, the code will look like:
if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420)
need_scaling = true /* which is wrong, as this function call is
for plane scalar, and scaler_user = scaler */
if (crtc_state->hdmi_output == DRM_HDMI_OUTPUT_YCBCR420 &&
scaler_user == HDMI_OUT)
need_scaling = true /* this is correct, as I want to consume scalar only
when both the conditions are true.
I meant to do just
if (scaler_user == SKL_HDMI_OUTPUT_INDEX)
...;
I failed to notice the ambiguity in abbreviating to just HDMI_OUTPUT. But the
caller shouldn't request it if it doesn't need it, right?
I agree, I dont see a reason why not !
Will do the needful.
Regards
Shashank
+ /* YCBCR 444 -> 420 conversion needs a scaler */
+ need_scaling = true;
+
/*
* if plane is being disabled or scaler is no more required or force
detach
* - free scaler binded to this plane/crtc
@@ -4673,6 +4678,26 @@ skl_update_scaler(struct intel_crtc_state *crtc_state,
bool force_detach,
}
/**
+ * skl_update_scaler_hdmi_output - Stages update to scaler state for HDMI.
+ * HDMI YCBCR 420 output needs a scaler, for downsampling.
+ *
+ * @state: crtc's scaler state
+ *
+ * Return
+ * 0 - scaler_usage updated successfully
+ *error - requested scaling cannot be supported or other error condition
+ */
+int skl_update_scaler_crtc_hdmi_output(struct intel_crtc_state *state)
+{
+ const struct drm_display_mode *mode = &state->base.adjusted_mode;
+
+ return skl_update_scaler(state, !state->bas
Re: [Freedreno] [PATCH] drm/msm: fix an integer overflow test
On Fri, Jun 30, 2017 at 10:59:15AM +0300, Dan Carpenter wrote:
> We recently added an integer overflow check but it needs an additional
> tweak to work properly on 32 bit systems.
>
> The problem is that we're doing the right hand side of the assignment as
> type unsigned long so the max it will have an integer overflow instead
> of being larger than SIZE_MAX. That means the "sz > SIZE_MAX" condition
> is never true even on 32 bit systems. We need to first cast it to u64
> and then do the math.
>
> Fixes: 4a630fadbb29 ("drm/msm: Fix potential buffer overflow issue")
> Signed-off-by: Dan Carpenter
Indeed. Thanks for the catch.
Acked-by: Jordan Crouse
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 6bfca7470141..8095658e8cb4 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -34,8 +34,8 @@ static struct msm_gem_submit *submit_create(struct
> drm_device *dev,
> struct msm_gpu *gpu, uint32_t nr_bos, uint32_t nr_cmds)
> {
> struct msm_gem_submit *submit;
> - uint64_t sz = sizeof(*submit) + (nr_bos * sizeof(submit->bos[0])) +
> - (nr_cmds * sizeof(submit->cmd[0]));
> + uint64_t sz = sizeof(*submit) + ((u64)nr_bos * sizeof(submit->bos[0])) +
> + ((u64)nr_cmds * sizeof(submit->cmd[0]));
>
> if (sz > SIZE_MAX)
> return NULL;
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/vblank: Introduce drm_crtc_vblank_get_accurate()
On Fri, Jun 30, 2017 at 05:18:41PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Add drm_crtc_vblank_get_accurate() which is the same as
> drm_crtc_vblank_get() except that it will forcefully update the
> the current vblank counter/timestamp value even if the interrupt
> is already enabled.
>
> And we'll switch drm_wait_one_vblank() over to using it to make sure it
> will really wait at least one vblank even if it races with the irq
> handler.
>
> Cc: Daniel Vetter
> Signed-off-by: Ville Syrjälä
Hm, I just thought of doing an
s/drm_vblank_count/drm_crtc_accurate_vblank_count/ in
drm_crtc_arm_vblank_event.
What does this buy us above&beyond the other patch? And why isn't
vblank_get accurate always?
/me confused
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_vblank.c | 37 -
> include/drm/drm_vblank.h | 1 +
> 2 files changed, 33 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 823c853de052..c57383b8753b 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -955,7 +955,8 @@ static int drm_vblank_enable(struct drm_device *dev,
> unsigned int pipe)
> return ret;
> }
>
> -static int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
> +static int drm_vblank_get(struct drm_device *dev, unsigned int pipe,
> + bool force_update)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> unsigned long irqflags;
> @@ -975,6 +976,10 @@ static int drm_vblank_get(struct drm_device *dev,
> unsigned int pipe)
> if (!vblank->enabled) {
> atomic_dec(&vblank->refcount);
> ret = -EINVAL;
> + } else if (force_update) {
> + spin_lock(&dev->vblank_time_lock);
> + drm_update_vblank_count(dev, pipe, false);
> + spin_unlock(&dev->vblank_time_lock);
> }
> }
> spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> @@ -994,10 +999,32 @@ static int drm_vblank_get(struct drm_device *dev,
> unsigned int pipe)
> */
> int drm_crtc_vblank_get(struct drm_crtc *crtc)
> {
> - return drm_vblank_get(crtc->dev, drm_crtc_index(crtc));
> + return drm_vblank_get(crtc->dev, drm_crtc_index(crtc), false);
> }
> EXPORT_SYMBOL(drm_crtc_vblank_get);
>
> +/**
> + * drm_crtc_vblank_get_accurate - get a reference count on vblank events and
> + *make sure the counter is uptodate
> + * @crtc: which CRTC to own
> + *
> + * Acquire a reference count on vblank events to avoid having them disabled
> + * while in use.
> + *
> + * Also make sure the current vblank counter is value is fully up to date
> + * even if we're already past the start of vblank but the irq hasn't fired
> + * yet, which may be the case with some hardware that raises the interrupt
> + * only some time after the start of vblank.
> + *
> + * Returns:
> + * Zero on success or a negative error code on failure.
> + */
> +int drm_crtc_vblank_get_accurate(struct drm_crtc *crtc)
> +{
> + return drm_vblank_get(crtc->dev, drm_crtc_index(crtc), true);
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_get_accurate);
> +
> static void drm_vblank_put(struct drm_device *dev, unsigned int pipe)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> @@ -1053,7 +1080,7 @@ void drm_wait_one_vblank(struct drm_device *dev,
> unsigned int pipe)
> if (WARN_ON(pipe >= dev->num_crtcs))
> return;
>
> - ret = drm_vblank_get(dev, pipe);
> + ret = drm_vblank_get(dev, pipe, true);
> if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", pipe, ret))
> return;
>
> @@ -1248,7 +1275,7 @@ static void drm_legacy_vblank_pre_modeset(struct
> drm_device *dev,
>*/
> if (!vblank->inmodeset) {
> vblank->inmodeset = 0x1;
> - if (drm_vblank_get(dev, pipe) == 0)
> + if (drm_vblank_get(dev, pipe, false) == 0)
> vblank->inmodeset |= 0x2;
> }
> }
> @@ -1448,7 +1475,7 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
> *data,
> return 0;
> }
>
> - ret = drm_vblank_get(dev, pipe);
> + ret = drm_vblank_get(dev, pipe, false);
> if (ret) {
> DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n",
> pipe, ret);
> return ret;
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 7fba9efe4951..5629e7841318 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -162,6 +162,7 @@ void drm_crtc_arm_vblank_event(struct drm_crtc *crtc,
> bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
> bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
> int drm_crtc_vblank_get(struct drm_crtc *crtc);
> +int drm_crtc_vblank_get_accurate(struct drm_crtc *crtc);
>
Re: [PATCH v4 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 04:53:19PM +0300, Ville Syrjälä wrote: > On Fri, Jun 30, 2017 at 03:35:03PM +0200, Daniel Vetter wrote: > > On Thu, Jun 29, 2017 at 10:26:08PM +0300, Ville Syrjälä wrote: > > > On Thu, Jun 29, 2017 at 06:57:30PM +0100, Chris Wilson wrote: > > > > Quoting ville.syrj...@linux.intel.com (2017-06-29 15:36:42) > > > > > From: Ville Syrjälä > > > > > > > > > > Introduce an rw_semaphore to protect the display commits. All normal > > > > > commits use down_read() and hence can proceed in parallel, but GPU > > > > > reset > > > > > will use down_write() making sure no other commits are in progress > > > > > when > > > > > we have to pull the plug on the display engine on pre-g4x platforms. > > > > > There are no modeset/gem locks taken inside > > > > > __intel_atomic_commit_tail() > > > > > itself, and we wait for all dependencies before the down_read(), and > > > > > thus there is no chance of deadlocks with this scheme. > > > > > > > > This matches what I thought should be done (I didn't think of using > > > > rwsem just a mutex, nice touch). The point I got stuck on was what > > > > should be done after the reset? I expected another modeset to return the > > > > state back or otherwise the inflight would get confused? > > > > > > I guess that can happen. For instance, if we have a crtc_enable() in > > > flight, > > > and then we do a reset before it gets committed we would end up doing > > > crtc_enable() twice in a row without a crtc_disable in between. For page > > > flips and such this shouldn't be a big deal in general. > > > > atomic commits are ordered. You have to wait for the previous ones to > > complete before you do a new one. If you don't do that, then all hell > > breaks loose. > > What we're effectively doing here is inserting two new commits in > the middle of the timeline, one to disable everything, and another > one to re-enable everything. At the end of the the re-enable we would > want the hardware state should match exactly what was there before > the disable, hence any commits still in the timeline should work > correctly. That is, their old_state will match the hardware state > when it comes time to commit them. > > But we can only do that properly after we start to track the committed > state. Without that tracking we can get into trouble wrt. the > hardware state not matching the old state when it comes time to > commit the new state. Yeah, but my point is that this here is an extremely fancy and fragile (and afaics in this form, incomplete) fix for something that in the past was fixed much, much simpler. Why do we need this massive amount of complexity now? Who's asking for all this (we don't even have anyone yet asking for fully queued atomic commits, much less on gen4)? I mean rewriting the entire modeset code is fun and all, but for gen3-4? > > What you really can't do with atomic (without rewriting everything once > > more) is cancel a commit. Pre-atomic we could do that on gen4 since the > > mmio flips died with the gpu, but that's the one design change we need to > > cope with (plus TDR insisting it can't force-complete requests anymore). > > > > > > > During reset we should be recommiting the state that was committed > > > > > last. > > > > > But for now we'll settle for recommiting the last state for each > > > > > object. > > > > > > > > Ah, I guess that explains the above. What is the complication with > > > > restoring the current state as opposed to the next state? > > > > > > Well the main thing is just tracking which is the current state. That > > > just needs refactoring the .atomic_duplicate_state() calling convention > > > across the whole tree so that we can then duplicate the committed state > > > rather than the latest state. > > > > > > Also due to the commit_hw_done() being potentially done after the > > > modeset locks have been dropped, I don't think we can be certain > > > of it getting called in the same order as swap_state(), hence > > > when we track the committed state in commit_hw_done() we'll have > > > to have some way to figure out if our new state is in fact the > > > latest committed state for each object or if the calls got > > > reordered. We don't insert any dependencies between two commits > > > unless they touch the same active crtc, thus this reordering > > > seems very much possible. Dunno if we should add some way to add > > > such dependeencies whenever the same object is part of two otherwise > > > independent commits, or if we just want to try and work with the > > > reordered calls. My idea for the latter was some kind of seqno/age > > > counter on the object states that allows me to recongnize which > > > state is more recent. The object states aren't refcounted so hanging > > > on to the wrong pointer could cause an oops the next time we have to > > > perform a GPU reset. > > > > Atomic commits are strongly ordered on a given CRTC, so stuff can't be > > out-of-order within one. Across them the i
Re: [Mesa-dev] [PATCH 2/4] intel: Add Cannonlake PCI IDs for Y-skus.
series pushed to libdrm. Thanks for the review.
On Thu, Jun 29, 2017 at 3:16 PM, Clint Taylor
wrote:
> Reviewed-by: Clinton Taylor
>
> -Clint
>
>
>
>
> On 06/29/2017 02:34 PM, Rodrigo Vivi wrote:
>>
>> By the Spec all CNL Y skus are 2+2, i.e. GT2.
>>
>> This is a copy of merged i915's
>> commit 95578277cbdb ("drm/i915/cnl: Add Cannonlake PCI IDs for Y-skus.")
>>
>> v2: Add kernel commit id for reference.
>>
>> Cc: Anusha Srivatsa
>> Cc: Clinton Taylor
>> Signed-off-by: Rodrigo Vivi
>> ---
>> intel/intel_chipset.h | 16 +++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h
>> index e6b49d7..37579c6 100644
>> --- a/intel/intel_chipset.h
>> +++ b/intel/intel_chipset.h
>> @@ -237,6 +237,12 @@
>> #define PCI_CHIP_CANNONLAKE_U_GT2_1 0x5A5A
>> #define PCI_CHIP_CANNONLAKE_U_GT2_2 0x5A42
>> #define PCI_CHIP_CANNONLAKE_U_GT2_3 0x5A4A
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_00x5A51
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_10x5A59
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_20x5A41
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_30x5A49
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_40x5A71
>> +#define PCI_CHIP_CANNONLAKE_Y_GT2_50x5A79
>> #define IS_MOBILE(devid)((devid) == PCI_CHIP_I855_GM || \
>> (devid) == PCI_CHIP_I915_GM || \
>> @@ -501,12 +507,20 @@
>> IS_GEN8(dev) || \
>> IS_GEN9(dev))
>> +#define IS_CNL_Y(devid) ((devid) ==
>> PCI_CHIP_CANNONLAKE_Y_GT2_0 || \
>> +(devid) == PCI_CHIP_CANNONLAKE_Y_GT2_1 ||
>> \
>> +(devid) == PCI_CHIP_CANNONLAKE_Y_GT2_2 ||
>> \
>> +(devid) == PCI_CHIP_CANNONLAKE_Y_GT2_3 ||
>> \
>> +(devid) == PCI_CHIP_CANNONLAKE_Y_GT2_4 ||
>> \
>> +(devid) == PCI_CHIP_CANNONLAKE_Y_GT2_5)
>> +
>> #define IS_CNL_U(devid) ((devid) ==
>> PCI_CHIP_CANNONLAKE_U_GT2_0 || \
>> (devid) == PCI_CHIP_CANNONLAKE_U_GT2_1 ||
>> \
>> (devid) == PCI_CHIP_CANNONLAKE_U_GT2_2 ||
>> \
>> (devid) == PCI_CHIP_CANNONLAKE_U_GT2_3)
>> -#define IS_CANNONLAKE(devid) (IS_CNL_U(devid))
>> +#define IS_CANNONLAKE(devid) (IS_CNL_U(devid) || \
>> +IS_CNL_Y(devid))
>> #define IS_GEN10(devid) (IS_CANNONLAKE(devid))
>>
>
>
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
Quoting Daniel Vetter (2017-06-30 16:30:59) > On Fri, Jun 30, 2017 at 04:53:19PM +0300, Ville Syrjälä wrote: > > On Fri, Jun 30, 2017 at 03:35:03PM +0200, Daniel Vetter wrote: > > > On Thu, Jun 29, 2017 at 10:26:08PM +0300, Ville Syrjälä wrote: > > > > On Thu, Jun 29, 2017 at 06:57:30PM +0100, Chris Wilson wrote: > > > > > Quoting ville.syrj...@linux.intel.com (2017-06-29 15:36:42) > > > > > > From: Ville Syrjälä > > > > > > > > > > > > Introduce an rw_semaphore to protect the display commits. All normal > > > > > > commits use down_read() and hence can proceed in parallel, but GPU > > > > > > reset > > > > > > will use down_write() making sure no other commits are in progress > > > > > > when > > > > > > we have to pull the plug on the display engine on pre-g4x platforms. > > > > > > There are no modeset/gem locks taken inside > > > > > > __intel_atomic_commit_tail() > > > > > > itself, and we wait for all dependencies before the down_read(), and > > > > > > thus there is no chance of deadlocks with this scheme. > > > > > > > > > > This matches what I thought should be done (I didn't think of using > > > > > rwsem just a mutex, nice touch). The point I got stuck on was what > > > > > should be done after the reset? I expected another modeset to return > > > > > the > > > > > state back or otherwise the inflight would get confused? > > > > > > > > I guess that can happen. For instance, if we have a crtc_enable() in > > > > flight, > > > > and then we do a reset before it gets committed we would end up doing > > > > crtc_enable() twice in a row without a crtc_disable in between. For page > > > > flips and such this shouldn't be a big deal in general. > > > > > > atomic commits are ordered. You have to wait for the previous ones to > > > complete before you do a new one. If you don't do that, then all hell > > > breaks loose. > > > > What we're effectively doing here is inserting two new commits in > > the middle of the timeline, one to disable everything, and another > > one to re-enable everything. At the end of the the re-enable we would > > want the hardware state should match exactly what was there before > > the disable, hence any commits still in the timeline should work > > correctly. That is, their old_state will match the hardware state > > when it comes time to commit them. > > > > But we can only do that properly after we start to track the committed > > state. Without that tracking we can get into trouble wrt. the > > hardware state not matching the old state when it comes time to > > commit the new state. > > Yeah, but my point is that this here is an extremely fancy and fragile > (and afaics in this form, incomplete) fix for something that in the past > was fixed much, much simpler. Why do we need this massive amount of > complexity now? Who's asking for all this (we don't even have anyone yet > asking for fully queued atomic commits, much less on gen4)? It was never "fixed", it was always borked; broken by design. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 03:30:33PM +0200, Daniel Vetter wrote: > On Fri, Jun 30, 2017 at 03:25:58PM +0200, Daniel Vetter wrote: > > On Thu, Jun 29, 2017 at 04:49:48PM +0300, ville.syrj...@linux.intel.com > > wrote: > > > From: Ville Syrjälä > > > > > > Introduce an rw_semaphore to protect the display commits. All normal > > > commits use down_read() and hence can proceed in parallel, but GPU reset > > > will use down_write() making sure no other commits are in progress when > > > we have to pull the plug on the display engine on pre-g4x platforms. > > > There are no modeset/gem locks taken inside __intel_atomic_commit_tail() > > > itself, and we wait for all dependencies before the down_read(), and > > > thus there is no chance of deadlocks with this scheme. > > > > How does this solve the deadlock? Afaiui the deadlock is that the gpu > > reset stopped unconditionally completing all requests before it did > > anything else, including anything with the hw or taking modeset locks. > > > > This ensured that any outstanding flips (we only had pageflips, no atomic > > ones back then) could complete (although maybe displaying garbage). The > > only thing we had to do was grab the locks (to avoid concurrent modesets) > > and then we could happily nuke the hw (since the flips where lost in the > > CS anyway), and restore it afterwards. > > > > Then the TDR rewrite came around and broke that, which now makes atomic > > time out waiting for the gpu to complete (because the gpu reset waits for > > the modeset to complete first). Which means if you want to fix this > > without breaking TDR, then you need to cancel the pending atomic commits. > > That seems somewhat what you're attempting here with trying to figure out > > what the latest hw-committed step is (but that function gets it wrong), > > and lots more locking and stuff on top. > > > > Why exactly can't we just go back to the old world of force-completing > > dead requests on gen4 and earlier? That would be tons simpler imo instead > > of throwing a pile of hacks (which really needs a complete rewrite of the > > atomic code in i915) in as a work-around. We didn't have TDR on gen4 and > > earlier for years, going back to that isn't going to hurt anyone. > > > > Making working gen4 gpu reset contigent on cancellable atomic commits and > > the full commit queue is imo nuts. > > And if the GEM folks insist the old behavior can't be restored, then we > just need a tailor-made get-out-of-jail card for gen4 gpu reset somewhere > in i915_sw_fence. Force-completing all render requests atomic updates > depend upon is imo the simplest solution to this, and we've had a driver > that worked like that for years. And it used to break all the time. I think we've had to fix it at least three times by now. So I tend to think it's better to fix it in a way that won't break so easily. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5] drm/sun4i: hdmi: Implement I2C adapter for A10s DDC bus
On Sat, Jul 01, 2017 at 12:14:44AM +1000, Jonathan Liu wrote: > Hi Chen-Yu and Maxime, > > On 30 June 2017 at 13:16, Chen-Yu Tsai wrote: > > On Fri, Jun 30, 2017 at 6:22 AM, Jonathan Liu wrote: > >> Hi Maxime, > >> > >> On 30 June 2017 at 01:56, Maxime Ripard > >> wrote: > >>> On Wed, Jun 28, 2017 at 08:39:33PM +1000, Jonathan Liu wrote: > >> + u32 int_status; > >> + u32 fifo_status; > >> + /* Read needs empty flag unset, write needs full flag unset */ > >> + u32 flag = read ? SUN4I_HDMI_DDC_FIFO_STATUS_EMPTY : > >> + SUN4I_HDMI_DDC_FIFO_STATUS_FULL; > >> + int ret; > >> + > >> + /* Wait until error or FIFO ready */ > >> + ret = readl_poll_timeout(hdmi->base + > >> SUN4I_HDMI_DDC_INT_STATUS_REG, > >> + int_status, > >> + is_err_status(int_status) || > >> + is_fifo_flag_unset(hdmi, &fifo_status, > >> flag), > >> + min(len, SUN4I_HDMI_DDC_FIFO_SIZE) * > >> byte_time, > >> + 10); > >> + > >> + if (is_err_status(int_status)) > >> + return -EIO; > >> + if (ret) > >> + return -ETIMEDOUT; > > > > Why not just have > > ret = readl_poll_timeout(hdmi->base + SUN4I_HDMI_DDC_FIFO_STATUS_REG, > > reg, > > !(reg & flag), 100, 10); > > > > if (ret < 0) > > if (is_err_status()) > > return -EIO; > > return ret; > > > > > > If I check error status after readl_poll_timeout and there is an error > (e.g. the I2C address does not have a corresponding device connected > or nothing connected to HDMI port) it will keep checking the fifo > status even though error bit is set in the int status and then timeout > after 100 ms. If it checks the int status register at the same time, > it will error after 100 nanoseconds. I don't want to introduce > unnecessary delays considering part of the reason for adding this > driver to make it more usable for non-standard use cases. > >>> > >>> Well, polling for 100ms doesn't seem great either. What was the > >>> rationale behind that timeout? > >>> > >> > >> When an error occurs one of the error bits will be set in the > >> INT_STATUS register so this is detected very quickly if I check the > >> INT_STATUS and FIFO_STATUS at the same time. The 100 ms timeout is in > >> case the I2C slave does clock stretching in which case the transfer > >> may take longer than the predicted time. > >> > >>> And we can also reverse the check and look at the INT_STATUS > >>> register. The errors will be there, and we can program the threshold > >>> we want in both directions and use the > >>> DDC_FIFO_Request_Interrupt_Status bit. > >>> > >> > >> I did try that when I was doing the v3 patch but I couldn't get it to > >> work as mentioned previously in the v3 patch discussion. I programmed > >> the FIFO_RX_TRIGGER_THRES and FIFO_TX_TRIGGER_THRES in DDC_FIFO_Ctrl > >> register at the same time as setting FIFO_Address_Clear but the > >> request interrupt status bit did not get updated to the appropriate > >> state that is consistent with the FIFO level and the thresholds. I did > >> try this several times for subsequent patch versions without success. > > > > The manual says "When FIFO level is above this value in read mode, DMA > > request and FIFO request interrupt are asserted if relative enable is on." > > > > Perhaps try enabling the interrupts? But if that were the case, wouldn't > > using interrupts instead of polling be better? > > > > ChenYu > > > > I managed to get the thresholds working so switching to using > interrupts instead of polling will be my next goal. I'd say that we can do it using polling first, and then move to interrupts if needed. And that should definitely be part of a separate patch. This one is already pretty invasive. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/fb-helper: Restore first connection behaviour on deferred setup
Prior to commit b0aa06e9a7fd ("drm/fb-helper: Support deferred setup"),
if no output is connected at framebuffer setup time, we get a default
1024x768 mode that is going to be used when we first connect a monitor.
After the commit, on first connection after deferred setup, we probe
the monitor and get the preferred resolution, but no mode get set
because the drm_fb_helper_hotplug_event() function returns early
when the setup has been deferred. That is different from what happens
on a second re-connect of the monitor, when the native mode get set.
Create a more consistent behaviour by checking in the
drm_fb_helper_hotplug_event() function if the deferred setup is still
active. If not, that means we now have a valid framebuffer that can be
used for setting the correct mode.
Fixes: b0aa06e9a7fd ("drm/fb-helper: Support deferred setup")
Signed-off-by: Liviu Dudau
Cc: Daniel Vetter
---
drivers/gpu/drm/drm_fb_helper.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d833eb2320d1..bb7b44d284ec 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2444,6 +2444,7 @@ static int __drm_fb_helper_initial_config(struct
drm_fb_helper *fb_helper,
if (ret == -EAGAIN) {
fb_helper->preferred_bpp = bpp_sel;
fb_helper->deferred_setup = true;
+ ret = 0;
}
mutex_unlock(&fb_helper->lock);
@@ -2565,7 +2566,13 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper
*fb_helper)
if (fb_helper->deferred_setup) {
err = __drm_fb_helper_initial_config(fb_helper,
fb_helper->preferred_bpp);
- return err;
+ /*
+* __drm_fb_helper_initial_config can change deferred_setup,
+* if 'false' that means we can go ahead with the rest of
+* the setup as normal
+*/
+ if (fb_helper->deferred_setup)
+ return err;
}
if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
--
2.13.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/6] drm/i915: update cursors asynchronously through atomic
From: Gustavo Padovan
Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
intel_legacy_cursor_update() did but through atomic.
v4:
- call drm_atomic_helper_async_check() from the check hook
v3:
- set correct vma to new state for cleanup
- move size checks back to drivers (Ville Syrjälä)
v2:
- move fb setting to core and use new state (Eric Anholt)
Cc: Daniel Vetter
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 73 ++
drivers/gpu/drm/i915/intel_display.c | 152 ++
2 files changed, 100 insertions(+), 125 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 4325cb0..e9fa7ca 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -230,11 +230,84 @@ static void intel_plane_atomic_update(struct drm_plane
*plane,
}
}
+static int intel_plane_atomic_async_check(struct drm_plane *plane,
+ struct drm_plane_state *state)
+{
+ struct drm_crtc *crtc = plane->state->crtc;
+ struct drm_crtc_state *crtc_state = crtc->state;
+
+ if (plane->type != DRM_PLANE_TYPE_CURSOR)
+ return -EINVAL;
+
+ /*
+* When crtc is inactive or there is a modeset pending,
+* wait for it to complete in the slowpath
+*/
+ if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
+ return -EINVAL;
+
+ /*
+* If any parameters change that may affect watermarks,
+* take the slowpath. Only changing fb or position should be
+* in the fastpath.
+*/
+ if (plane->state->crtc != state->crtc ||
+ plane->state->src_w != state->src_w ||
+ plane->state->src_h != state->src_h ||
+ plane->state->crtc_w != state->crtc_w ||
+ plane->state->crtc_h != state->crtc_h ||
+ !plane->state->fb != !state->fb)
+ return -EINVAL;
+
+ return 0;
+}
+
+static void intel_plane_atomic_async_update(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_crtc *crtc = plane->state->crtc;
+ struct drm_framebuffer *old_fb;
+ struct i915_vma *old_vma;
+
+ old_vma = to_intel_plane_state(plane->state)->vma;
+ old_fb = plane->state->fb;
+
+ i915_gem_track_fb(intel_fb_obj(old_fb), intel_fb_obj(new_state->fb),
+ intel_plane->frontbuffer_bit);
+
+ plane->state->src_x = new_state->src_x;
+ plane->state->src_y = new_state->src_y;
+ plane->state->crtc_x = new_state->crtc_x;
+ plane->state->crtc_y = new_state->crtc_y;
+ plane->state->fb = new_state->fb;
+ *to_intel_plane_state(plane->state) = *to_intel_plane_state(new_state);
+
+ to_intel_plane_state(new_state)->vma = old_vma;
+ new_state->fb = old_fb;
+
+ if (plane->state->visible) {
+ trace_intel_update_plane(plane, to_intel_crtc(crtc));
+ intel_plane->update_plane(intel_plane,
+ to_intel_crtc_state(crtc->state),
+ to_intel_plane_state(plane->state));
+ } else {
+ trace_intel_disable_plane(plane, to_intel_crtc(crtc));
+ intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
+ }
+
+ mutex_lock(&plane->dev->struct_mutex);
+ intel_cleanup_plane_fb(plane, new_state);
+ mutex_unlock(&plane->dev->struct_mutex);
+}
+
const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
.prepare_fb = intel_prepare_plane_fb,
.cleanup_fb = intel_cleanup_plane_fb,
.atomic_check = intel_plane_atomic_check,
.atomic_update = intel_plane_atomic_update,
+ .atomic_async_check = intel_plane_atomic_async_check,
+ .atomic_async_update = intel_plane_atomic_async_update,
};
/**
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4e03ca6..8817222 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12764,6 +12764,9 @@ static int intel_atomic_check(struct drm_device *dev,
if (ret)
return ret;
+ if (state->legacy_cursor_update)
+ state->async_update = !drm_atomic_helper_async_check(dev,
state);
+
intel_fbc_choose_crtc(dev_priv, state);
return calc_watermark_data(state);
}
@@ -13236,6 +13239,26 @@ static int intel_atomic_commit(struct drm_device *dev,
struct drm_i915_private *dev_priv = to_i915(dev);
int ret = 0;
+ /*
+* The atomic async update fast path takes care
+*
[PATCH v4 0/6] drm: add asynchrounous plane update
From: Gustavo Padovan Hi, Follow up after Daniel's comments. Here I move the common async code to drm_atomic_helper.c. i915 and msm now have to call the drm_atomic_helper_async_check() themselves. Please review! Thanks. Gustavo Gustavo Padovan (6): drm/atomic: initial support for asynchronous plane update drm/i915: update cursors asynchronously through atomic drm/i915: remove intel_cursor_plane_funcs drm/msm: update cursors asynchronously through atomic drm/msm: remove mdp5_cursor_plane_funcs drm/vc4: update cursors asynchronously through atomic drivers/gpu/drm/drm_atomic_helper.c | 122 + drivers/gpu/drm/i915/intel_atomic_plane.c | 73 + drivers/gpu/drm/i915/intel_display.c | 163 +--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 175 +++--- drivers/gpu/drm/msm/msm_atomic.c | 9 ++ drivers/gpu/drm/vc4/vc4_kms.c | 20 drivers/gpu/drm/vc4/vc4_plane.c | 128 +- include/drm/drm_atomic.h | 2 + include/drm/drm_atomic_helper.h | 4 + include/drm/drm_modeset_helper_vtables.h | 50 + 10 files changed, 423 insertions(+), 323 deletions(-) -- 2.9.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/6] drm/atomic: initial support for asynchronous plane update
From: Gustavo Padovan
In some cases, like cursor updates, it is interesting to update the
plane in an asynchronous fashion to avoid big delays. The current queued
update could be still waiting for a fence to signal and thus block any
subsequent update until its scan out. In cases like this if we update the
cursor synchronously through the atomic API it will cause significant
delays that would even be noticed by the final user.
This patch creates a fast path to jump ahead the current queued state and
do single planes updates without going through all atomic steps in
drm_atomic_helper_commit(). We take this path for legacy cursor updates.
For now only single plane updates are supported, but we plan to support
multiple planes updates and async PageFlips through this interface as well
in the near future.
v6: - move check code to drm_atomic_helper.c (Daniel Vetter)
v5:
- improve comments (Eric Anholt)
v4:
- fix state->crtc NULL check (Archit Taneja)
v3:
- fix iteration on the wrong crtc state
- put back code to forbid updates if there is a queued update for
the same plane (Ville Syrjälä)
- move size checks back to drivers (Ville Syrjälä)
- move ASYNC_UPDATE flag addition to its own patch (Ville Syrjälä)
v2:
- allow updates even if there is a queued update for the same
plane.
- fixes on the documentation (Emil Velikov)
- unconditionally call ->atomic_async_update (Emil Velikov)
- check for ->atomic_async_update earlier (Daniel Vetter)
- make ->atomic_async_check() the last step (Daniel Vetter)
- add ASYNC_UPDATE flag (Eric Anholt)
- update state in core after ->atomic_async_update (Eric Anholt)
- update docs (Eric Anholt)
Cc: Daniel Vetter
Cc: Rob Clark
Cc: Eric Anholt
Signed-off-by: Gustavo Padovan
Reviewed-by: Archit Taneja (v5)
Acked-by: Eric Anholt (v5)
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic_helper.c | 122 +++
include/drm/drm_atomic.h | 2 +
include/drm/drm_atomic_helper.h | 4 +
include/drm/drm_modeset_helper_vtables.h | 50 +
4 files changed, 178 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 23e4661..4f6e529 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -795,6 +795,9 @@ int drm_atomic_helper_check(struct drm_device *dev,
if (ret)
return ret;
+ if (state->legacy_cursor_update)
+ state->async_update = !drm_atomic_helper_async_check(dev,
state);
+
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_check);
@@ -1353,6 +1356,114 @@ static void commit_work(struct work_struct *work)
}
/**
+ * drm_atomic_helper_async_check - check if state can be commited
asynchronously
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * This helper will check if it is possible to commit the state asynchronously.
+ * Async commits are not supposed to swap the states like normal sync commits
+ * but just do in-place changes on the current state.
+ *
+ * It will return 0 if the commit can happen in an asynchronous fashion or
error
+ * if not. Note that error just mean it can't be commited asynchronously, if it
+ * fails the commit should be treated like a normal synchronous commit.
+ */
+int drm_atomic_helper_async_check(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ struct drm_crtc_commit *commit;
+ struct drm_plane *__plane, *plane = NULL;
+ struct drm_plane_state *__plane_state, *plane_state = NULL;
+ const struct drm_plane_helper_funcs *funcs;
+ int i, j, n_planes = 0;
+
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ if (drm_atomic_crtc_needs_modeset(crtc_state))
+ return -EINVAL;
+ }
+
+ for_each_new_plane_in_state(state, __plane, __plane_state, i) {
+ n_planes++;
+ plane = __plane;
+ plane_state = __plane_state;
+ }
+
+ /* FIXME: we support only single plane updates for now */
+ if (!plane || n_planes != 1)
+ return -EINVAL;
+
+ if (!plane_state->crtc)
+ return -EINVAL;
+
+ funcs = plane->helper_private;
+ if (!funcs->atomic_async_update)
+ return -EINVAL;
+
+ if (plane_state->fence)
+ return -EINVAL;
+
+ /*
+* Don't do an async update if there is an outstanding commit modifying
+* the plane. This prevents our async update's changes from getting
+* overridden by a previous synchronous update's state.
+*/
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ if (plane->crtc != crtc)
+
[PATCH v4 3/6] drm/i915: remove intel_cursor_plane_funcs
From: Gustavo Padovan
After converting legacy cursor updates to atomic async commits
intel_cursor_plane_funcs just duplicates intel_plane_funcs now.
Cc: Daniel Vetter
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/i915/intel_display.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8817222..73d2508 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13610,17 +13610,6 @@ const struct drm_plane_funcs intel_plane_funcs = {
.atomic_destroy_state = intel_plane_destroy_state,
};
-static const struct drm_plane_funcs intel_cursor_plane_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
- .destroy = intel_plane_destroy,
- .set_property = drm_atomic_helper_plane_set_property,
- .atomic_get_property = intel_plane_atomic_get_property,
- .atomic_set_property = intel_plane_atomic_set_property,
- .atomic_duplicate_state = intel_plane_duplicate_state,
- .atomic_destroy_state = intel_plane_destroy_state,
-};
-
static struct intel_plane *
intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
{
@@ -13782,7 +13771,7 @@ intel_cursor_plane_create(struct drm_i915_private
*dev_priv,
cursor->cursor.size = ~0;
ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
- 0, &intel_cursor_plane_funcs,
+ 0, &intel_plane_funcs,
intel_cursor_formats,
ARRAY_SIZE(intel_cursor_formats),
DRM_PLANE_TYPE_CURSOR,
--
2.9.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 5/6] drm/msm: remove mdp5_cursor_plane_funcs
From: Gustavo Padovan
After converting legacy cursor updates to atomic async commits
mdp5_cursor_plane_funcs just duplicates mdp5_plane_funcs now.
Cc: Rob Clark
Cc: Archit Taneja
Signed-off-by: Gustavo Padovan
Tested-by: Archit Taneja
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 26 +++---
1 file changed, 3 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 60680e8..82aa887 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -246,19 +246,6 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
.atomic_print_state = mdp5_plane_atomic_print_state,
};
-static const struct drm_plane_funcs mdp5_cursor_plane_funcs = {
- .update_plane = drm_atomic_helper_update_plane,
- .disable_plane = drm_atomic_helper_disable_plane,
- .destroy = mdp5_plane_destroy,
- .set_property = drm_atomic_helper_plane_set_property,
- .atomic_set_property = mdp5_plane_atomic_set_property,
- .atomic_get_property = mdp5_plane_atomic_get_property,
- .reset = mdp5_plane_reset,
- .atomic_duplicate_state = mdp5_plane_duplicate_state,
- .atomic_destroy_state = mdp5_plane_destroy_state,
- .atomic_print_state = mdp5_plane_atomic_print_state,
-};
-
static int mdp5_plane_prepare_fb(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
@@ -1110,16 +1097,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
ARRAY_SIZE(mdp5_plane->formats), false);
- if (type == DRM_PLANE_TYPE_CURSOR)
- ret = drm_universal_plane_init(dev, plane, 0xff,
- &mdp5_cursor_plane_funcs,
- mdp5_plane->formats, mdp5_plane->nformats,
- type, NULL);
- else
- ret = drm_universal_plane_init(dev, plane, 0xff,
- &mdp5_plane_funcs,
- mdp5_plane->formats, mdp5_plane->nformats,
- type, NULL);
+ ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs,
+ mdp5_plane->formats,
+ mdp5_plane->nformats, type, NULL);
if (ret)
goto fail;
--
2.9.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 4/6] drm/msm: update cursors asynchronously through atomic
From: Gustavo Padovan
Add support to async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
mdp5_update_cursor_plane_legacy() did but through atomic.
v5: call drm_atomic_helper_async_check() from the check hook
v4: add missing atomic async commit call to msm_atomic_commit(Archit Taneja)
v3: move size checks back to drivers (Ville Syrjälä)
v2: move fb setting to core and use new state (Eric Anholt)
Cc: Rob Clark
Cc: Archit Taneja
Signed-off-by: Gustavo Padovan
Tested-by: Archit Taneja (v4)
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 151 +-
drivers/gpu/drm/msm/msm_atomic.c | 9 ++
2 files changed, 72 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index fe3a4de..60680e8 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -31,15 +31,6 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
struct drm_crtc *crtc, struct drm_framebuffer *fb,
struct drm_rect *src, struct drm_rect *dest);
-static int mdp5_update_cursor_plane_legacy(struct drm_plane *plane,
- struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h,
- struct drm_modeset_acquire_ctx *ctx);
-
static struct mdp5_kms *get_kms(struct drm_plane *plane)
{
struct msm_drm_private *priv = plane->dev->dev_private;
@@ -256,7 +247,7 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
};
static const struct drm_plane_funcs mdp5_cursor_plane_funcs = {
- .update_plane = mdp5_update_cursor_plane_legacy,
+ .update_plane = drm_atomic_helper_update_plane,
.disable_plane = drm_atomic_helper_disable_plane,
.destroy = mdp5_plane_destroy,
.set_property = drm_atomic_helper_plane_set_property,
@@ -489,11 +480,73 @@ static void mdp5_plane_atomic_update(struct drm_plane
*plane,
}
}
+static int mdp5_plane_atomic_async_check(struct drm_plane *plane,
+struct drm_plane_state *state)
+{
+ struct mdp5_plane_state *mdp5_state = to_mdp5_plane_state(state);
+ struct drm_crtc_state *crtc_state;
+
+ crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+ state->crtc);
+ if (WARN_ON(!crtc_state))
+ return -EINVAL;
+
+ if (!crtc_state->active)
+ return -EINVAL;
+
+ mdp5_state = to_mdp5_plane_state(state);
+
+ /* don't use fast path if we don't have a hwpipe allocated yet */
+ if (!mdp5_state->hwpipe)
+ return -EINVAL;
+
+ /* only allow changing of position(crtc x/y or src x/y) in fast path */
+ if (plane->state->crtc != state->crtc ||
+ plane->state->src_w != state->src_w ||
+ plane->state->src_h != state->src_h ||
+ plane->state->crtc_w != state->crtc_w ||
+ plane->state->crtc_h != state->crtc_h ||
+ !plane->state->fb ||
+ plane->state->fb != state->fb)
+ return -EINVAL;
+
+ return 0;
+}
+
+static void mdp5_plane_atomic_async_update(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ plane->state->src_x = new_state->src_x;
+ plane->state->src_y = new_state->src_y;
+ plane->state->crtc_x = new_state->crtc_x;
+ plane->state->crtc_y = new_state->crtc_y;
+
+ if (plane_enabled(new_state)) {
+ struct mdp5_ctl *ctl;
+ struct mdp5_pipeline *pipeline =
+ mdp5_crtc_get_pipeline(plane->crtc);
+ int ret;
+
+ ret = mdp5_plane_mode_set(plane, new_state->crtc, new_state->fb,
+ &new_state->src, &new_state->dst);
+ WARN_ON(ret < 0);
+
+ ctl = mdp5_crtc_get_ctl(new_state->crtc);
+
+ mdp5_ctl_commit(ctl, pipeline, mdp5_plane_get_flush(plane));
+ }
+
+ *to_mdp5_plane_state(plane->state) =
+ *to_mdp5_plane_state(new_state);
+}
+
static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
.prepare_fb = mdp5_plane_prepare_fb,
.cleanup_fb = mdp5_plane_cleanup_fb,
.atomic_check = mdp5_plane_atomic_check,
.atomic_update = mdp5_plane_atomic_update,
+ .atomic_async_check = mdp5_plane_atomic_async_check,
+ .atomic_async_update = mdp5_plane_atomic_async_update,
};
static void set_scanout_locked(struct mdp5_kms *mdp5_kms,
@@ -998,84 +1051,6 @@
[PATCH v4 6/6] drm/vc4: update cursors asynchronously through atomic
From: Gustavo Padovan
Add support for async updates of cursors by using the new atomic
interface for that. Basically what this commit does is do what
vc4_update_plane() did but through atomic.
v5: add missing call to vc4_plane_atomic_check() (Eric Anholt)
v4: add drm_atomic_helper_async() commit (Eric Anholt)
v3: move size checks back to drivers (Ville Syrjälä)
v2: move fb setting to core and use new state (Eric Anholt)
Cc: Eric Anholt
Signed-off-by: Gustavo Padovan
Reviewed-by: Eric Anholt
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/vc4/vc4_kms.c | 20 +++
drivers/gpu/drm/vc4/vc4_plane.c | 128
2 files changed, 71 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 27edae4..efd2656 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -95,6 +95,26 @@ static int vc4_atomic_commit(struct drm_device *dev,
struct vc4_dev *vc4 = to_vc4_dev(dev);
int ret;
+ if (state->async_update) {
+ ret = down_interruptible(&vc4->async_modeset);
+ if (ret)
+ return ret;
+
+ ret = drm_atomic_helper_prepare_planes(dev, state);
+ if (ret) {
+ up(&vc4->async_modeset);
+ return ret;
+ }
+
+ drm_atomic_helper_async_commit(dev, state);
+
+ drm_atomic_helper_cleanup_planes(dev, state);
+
+ up(&vc4->async_modeset);
+
+ return 0;
+ }
+
ret = drm_atomic_helper_setup_commit(state, nonblock);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
index 8853e9a..52bf74c 100644
--- a/drivers/gpu/drm/vc4/vc4_plane.c
+++ b/drivers/gpu/drm/vc4/vc4_plane.c
@@ -759,87 +759,41 @@ void vc4_plane_async_set_fb(struct drm_plane *plane,
struct drm_framebuffer *fb)
vc4_state->dlist[vc4_state->ptr0_offset] = addr;
}
-static int vc4_prepare_fb(struct drm_plane *plane,
- struct drm_plane_state *state)
+static int vc4_plane_atomic_async_check(struct drm_plane *plane,
+ struct drm_plane_state *state)
{
- struct vc4_bo *bo;
- struct dma_fence *fence;
+ if (plane != state->crtc->cursor)
+ return -EINVAL;
- if ((plane->state->fb == state->fb) || !state->fb)
- return 0;
+ if (!plane->state)
+ return -EINVAL;
- bo = to_vc4_bo(&drm_fb_cma_get_gem_obj(state->fb, 0)->base);
- fence = reservation_object_get_excl_rcu(bo->resv);
- drm_atomic_set_fence_for_plane(state, fence);
+ /* No configuring new scaling in the fast path. */
+ if (state->crtc_w != plane->state->crtc_w ||
+ state->crtc_h != plane->state->crtc_h ||
+ state->src_w != plane->state->src_w ||
+ state->src_h != plane->state->src_h) {
+ return -EINVAL;
+ }
return 0;
}
-static const struct drm_plane_helper_funcs vc4_plane_helper_funcs = {
- .atomic_check = vc4_plane_atomic_check,
- .atomic_update = vc4_plane_atomic_update,
- .prepare_fb = vc4_prepare_fb,
-};
-
-static void vc4_plane_destroy(struct drm_plane *plane)
-{
- drm_plane_helper_disable(plane);
- drm_plane_cleanup(plane);
-}
-
-/* Implements immediate (non-vblank-synced) updates of the cursor
- * position, or falls back to the atomic helper otherwise.
- */
-static int
-vc4_update_plane(struct drm_plane *plane,
-struct drm_crtc *crtc,
-struct drm_framebuffer *fb,
-int crtc_x, int crtc_y,
-unsigned int crtc_w, unsigned int crtc_h,
-uint32_t src_x, uint32_t src_y,
-uint32_t src_w, uint32_t src_h,
-struct drm_modeset_acquire_ctx *ctx)
+static void vc4_plane_atomic_async_update(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
{
- struct drm_plane_state *plane_state;
- struct vc4_plane_state *vc4_state;
-
- if (plane != crtc->cursor)
- goto out;
-
- plane_state = plane->state;
- vc4_state = to_vc4_plane_state(plane_state);
-
- if (!plane_state)
- goto out;
-
- /* No configuring new scaling in the fast path. */
- if (crtc_w != plane_state->crtc_w ||
- crtc_h != plane_state->crtc_h ||
- src_w != plane_state->src_w ||
- src_h != plane_state->src_h) {
- goto out;
- }
-
- if (fb != plane_state->fb) {
- drm_atomic_set_fb_for_plane(plane->state, fb);
- vc4_plane_async_set_fb(plane, fb);
- }
+ struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state);
- /* Set the cursor's position on the screen. Thi
Re: [PATCH] drm/fb-helper: Restore first connection behaviour on deferred setup
On Fri, Jun 30, 2017 at 6:51 PM, Liviu Dudau wrote:
> Prior to commit b0aa06e9a7fd ("drm/fb-helper: Support deferred setup"),
> if no output is connected at framebuffer setup time, we get a default
> 1024x768 mode that is going to be used when we first connect a monitor.
> After the commit, on first connection after deferred setup, we probe
> the monitor and get the preferred resolution, but no mode get set
> because the drm_fb_helper_hotplug_event() function returns early
> when the setup has been deferred. That is different from what happens
> on a second re-connect of the monitor, when the native mode get set.
>
> Create a more consistent behaviour by checking in the
> drm_fb_helper_hotplug_event() function if the deferred setup is still
> active. If not, that means we now have a valid framebuffer that can be
> used for setting the correct mode.
>
> Fixes: b0aa06e9a7fd ("drm/fb-helper: Support deferred setup")
> Signed-off-by: Liviu Dudau
> Cc: Daniel Vetter
I thought the analysis over irc was that fbcon picked a different
driver as it's console, and that's why nothing shows up on the malidp
output in the deferred case? That's mildly annoying, but iirc fbcon
has always been rather erratic in multi-gpu setups. Although I thought
that it would by default bind all fbdev drivers as consoles (and then
you need to rebind the right console driver, if the right Kconfig is
enabled, through sysfs).
Either way if the register_framebuffer() call in initial_config isn't
good enough, then we need to add the set_par in initial_config
unconditionally, not just in the deferred probe case. Just disable
fbcon entirely for testing, in that case even without deferred probing
nothing will show up.
I'd say if this is still needed in the single gpu case then we need to
investigate more, but for multi-gpu it is what it is (aka fbcon is not
great).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/vblank: Introduce drm_crtc_vblank_get_accurate()
On Fri, Jun 30, 2017 at 5:26 PM, Daniel Vetter wrote: > On Fri, Jun 30, 2017 at 05:18:41PM +0300, ville.syrj...@linux.intel.com wrote: >> From: Ville Syrjälä >> >> Add drm_crtc_vblank_get_accurate() which is the same as >> drm_crtc_vblank_get() except that it will forcefully update the >> the current vblank counter/timestamp value even if the interrupt >> is already enabled. >> >> And we'll switch drm_wait_one_vblank() over to using it to make sure it >> will really wait at least one vblank even if it races with the irq >> handler. >> >> Cc: Daniel Vetter >> Signed-off-by: Ville Syrjälä > > Hm, I just thought of doing an > s/drm_vblank_count/drm_crtc_accurate_vblank_count/ in > drm_crtc_arm_vblank_event. > > What does this buy us above&beyond the other patch? And why isn't > vblank_get accurate always? This also seems to have the risk of not fixing the arm_vblank_event race if someone puts the vblank_get_accurate outside of the critical section. Then we're back to the same old race. And since that means you need to have a vblank_get_accurate right before the arm_vblank_event in all cases, we might as well just put it into the helper. You wrote in the other thread putting it into arm_vblank_event might be wasteful, but I don't see any scenario where that could be the case ... Or do I miss something? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 5:44 PM, Ville Syrjälä wrote: >> And if the GEM folks insist the old behavior can't be restored, then we >> just need a tailor-made get-out-of-jail card for gen4 gpu reset somewhere >> in i915_sw_fence. Force-completing all render requests atomic updates >> depend upon is imo the simplest solution to this, and we've had a driver >> that worked like that for years. > > And it used to break all the time. I think we've had to fix it at least > three times by now. So I tend to think it's better to fix it in a way > that won't break so easily. Why exactly is making the atomic code massive more tricky the easy fix? That's the part I don't get. Yes it got broken a bunch because no one runs CI and everyone forgets that gen3/4 reset the display in gpu reset, but in the end we do have a depency loop, and either the modeset side or the render side needs to bail out and cancel it's async stuff (whether that's a request or a nonblocking flip/atomic commit doesn't matter). In my opinion, cancelling the request (even if we're clever and only cancel the requests for the modeset waiters, which isn't to hard to pull off) seems about the simplest option. Especially since we need that code anyway, even TDR can't safe everything and resubmit under all circumstances (at least the buggy batch can't be resubmitted). Cancelling any kind of atomic commit otoh looks like a lot more complexity. Why do you think this is the easier, or at least less fragile option? This patch series is full of FIXMEs, and even the more complete set seems to have a pile of holes. Plus we need to stop using obj->state, and I don't see any easy way to test for that (since the gen3/4 gpu reset case is the only corner cases that currently needs that). So not seeing how this is easier or more robust at all. What do I miss? Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/i915: Solve the GPU reset vs. modeset deadlocks with an rw_semaphore
On Fri, Jun 30, 2017 at 08:23:58PM +0200, Daniel Vetter wrote: > On Fri, Jun 30, 2017 at 5:44 PM, Ville Syrjälä > wrote: > >> And if the GEM folks insist the old behavior can't be restored, then we > >> just need a tailor-made get-out-of-jail card for gen4 gpu reset somewhere > >> in i915_sw_fence. Force-completing all render requests atomic updates > >> depend upon is imo the simplest solution to this, and we've had a driver > >> that worked like that for years. > > > > And it used to break all the time. I think we've had to fix it at least > > three times by now. So I tend to think it's better to fix it in a way > > that won't break so easily. > > Why exactly is making the atomic code massive more tricky the easy > fix? I don't see what this massive trickyness is. Compared to the rest of atomic what I have is absolutely trivial. Just the duplicate_committed_state() and the '.committed_state = foo' assignments in hw_done(). That's it really. > That's the part I don't get. Yes it got broken a bunch because no > one runs CI and everyone forgets that gen3/4 reset the display in gpu > reset, but in the end we do have a depency loop, and either the > modeset side or the render side needs to bail out and cancel it's > async stuff (whether that's a request or a nonblocking flip/atomic > commit doesn't matter). In my opinion, cancelling the request (even if > we're clever and only cancel the requests for the modeset waiters, > which isn't to hard to pull off) seems about the simplest option. > Especially since we need that code anyway, even TDR can't safe > everything and resubmit under all circumstances (at least the buggy > batch can't be resubmitted). > > Cancelling any kind of atomic commit otoh looks like a lot more > complexity. I'm not cancelling anything. > Why do you think this is the easier, or at least less > fragile option? This patch series is full of FIXMEs, and even the more > complete set seems to have a pile of holes. Plus we need to stop using > obj->state, and I don't see any easy way to test for that (since the > gen3/4 gpu reset case is the only corner cases that currently needs > that). We need to fix that stuff anyway if we ever want to queue up multiple commits for the same crtc. The stuff I have that is specific to this reset stuff is actually very simple. The rest is just fixing up the huge mess we've already made. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/2] radeon: use asic id table to get chipset name
Change-Id: I24b6624789d1a9dc0fd3a446b0e6f21ed5183ff2
Signed-off-by: Samuel Li
---
radeon/Makefile.am | 6 +++
radeon/Makefile.sources | 6 ++-
radeon/radeon_asic_id.c | 106
radeon/radeon_asic_id.h | 37 +
4 files changed, 153 insertions(+), 2 deletions(-)
create mode 100644 radeon/radeon_asic_id.c
create mode 100644 radeon/radeon_asic_id.h
diff --git a/radeon/Makefile.am b/radeon/Makefile.am
index e241531..69407bc 100644
--- a/radeon/Makefile.am
+++ b/radeon/Makefile.am
@@ -30,6 +30,12 @@ AM_CFLAGS = \
$(PTHREADSTUBS_CFLAGS) \
-I$(top_srcdir)/include/drm
+libdrmdatadir = @libdrmdatadir@
+ASIC_ID_TABLE_NUM_ENTRIES := $(shell egrep -ci '^[0-9a-f]{4},.*[0-9a-f]+,' \
+ $(top_srcdir)/data/amdgpu.ids)
+AM_CPPFLAGS = -DRADEON_ASIC_ID_TABLE=\"${libdrmdatadir}/amdgpu.ids\" \
+ -DRADEON_ASIC_ID_TABLE_NUM_ENTRIES=$(ASIC_ID_TABLE_NUM_ENTRIES)
+
libdrm_radeon_la_LTLIBRARIES = libdrm_radeon.la
libdrm_radeon_ladir = $(libdir)
libdrm_radeon_la_LDFLAGS = -version-number 1:0:1 -no-undefined
diff --git a/radeon/Makefile.sources b/radeon/Makefile.sources
index 1cf482a..8eaf1c6 100644
--- a/radeon/Makefile.sources
+++ b/radeon/Makefile.sources
@@ -4,7 +4,8 @@ LIBDRM_RADEON_FILES := \
radeon_cs_space.c \
radeon_bo.c \
radeon_cs.c \
- radeon_surface.c
+ radeon_surface.c \
+ radeon_asic_id.c
LIBDRM_RADEON_H_FILES := \
radeon_bo.h \
@@ -14,7 +15,8 @@ LIBDRM_RADEON_H_FILES := \
radeon_cs_gem.h \
radeon_bo_int.h \
radeon_cs_int.h \
- r600_pci_ids.h
+ r600_pci_ids.h \
+ radeon_asic_id.h
LIBDRM_RADEON_BOF_FILES := \
bof.c \
diff --git a/radeon/radeon_asic_id.c b/radeon/radeon_asic_id.c
new file mode 100644
index 000..b03502b
--- /dev/null
+++ b/radeon/radeon_asic_id.c
@@ -0,0 +1,106 @@
+/*
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+/**
+ * \file radeon_asic_id.c
+ *
+ * Implementation of chipset name lookup functions for radeon device
+ *
+ */
+
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+//#include
+//#include
+#include
+#include
+#include
+#include
+#include
+
+#include "xf86atomic.h"
+#include "util/util_asic_id.h"
+#include "radeon_asic_id.h"
+
+
+static pthread_mutex_t asic_id_mutex = PTHREAD_MUTEX_INITIALIZER;
+static struct util_asic_id *radeon_asic_ids;
+static atomic_t refcount;
+
+int radeon_asic_id_initialize(void)
+{
+ int r = 0;
+ pthread_mutex_lock(&asic_id_mutex);
+ if (radeon_asic_ids) {
+ atomic_inc(&refcount);
+ pthread_mutex_unlock(&asic_id_mutex);
+ return r;
+ }
+
+ r = util_parse_asic_ids(&radeon_asic_ids, RADEON_ASIC_ID_TABLE,
+ RADEON_ASIC_ID_TABLE_NUM_ENTRIES);
+ if (r) {
+ fprintf(stderr, "%s: Cannot parse ASIC IDs, 0x%x.",
+ __func__, r);
+ } else
+ atomic_inc(&refcount);
+
+ pthread_mutex_unlock(&asic_id_mutex);
+ return r;
+}
+
+void radeon_asic_id_deinitialize(void)
+{
+ const struct util_asic_id *id;
+
+ assert(atomic_read(&refcount) > 0);
+ pthread_mutex_lock(&asic_id_mutex);
+ if (atomic_dec_and_test(&refcount)) {
+ if (radeon_asic_ids) {
+ for (id = radeon_asic_ids; id->did; id++)
+ free(id->marketing_name);
+ free(radeon_asic_ids);
+ radeon_asic_ids = NULL;
+ }
+ }
+ pthread_mutex_unlock(&asic_id_mutex);
+}
+
+const char *radeon_get_marketing_name(uint32_t device_id, uint32_t pci_rev_id)
+{
+ const struct util_asic_id *id;
+
+ if (!radeon_asic_ids)
+ return NULL;
+
+ for (id = radeon_asic_ids; id->did; id++
[PATCH] drm/panel: simple: Skip error message on deferred probe
From: Fabio Estevam
When enable_gpio is provided via an I2C or SPI expander, it may not be
available when panel-simple probes leading to the following error:
panel-simple panel: failed to request GPIO: -517
As this error message is not very useful to the end user, skip printing
it in the case of deferred probe.
Signed-off-by: Fabio Estevam
---
drivers/gpu/drm/panel/panel-simple.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 474fa75..9469c4d 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -315,7 +315,8 @@ static int panel_simple_probe(struct device *dev, const
struct panel_desc *desc)
GPIOD_OUT_LOW);
if (IS_ERR(panel->enable_gpio)) {
err = PTR_ERR(panel->enable_gpio);
- dev_err(dev, "failed to request GPIO: %d\n", err);
+ if (err != -EPROBE_DEFER)
+ dev_err(dev, "failed to request GPIO: %d\n", err);
return err;
}
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] intel/intel_chipset: Move IS_9XX below IS_GEN10.
No functional change. Just organizing the code so it gets clear for future platforms. Paulo deserves credits becuase he was the one that just noticed this IS_9XX was in the wrong position after CNL patches got introduced. Cc: Paulo Zanoni Signed-off-by: Rodrigo Vivi --- intel/intel_chipset.h | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/intel/intel_chipset.h b/intel/intel_chipset.h index 770d21f..3ff59ad 100644 --- a/intel/intel_chipset.h +++ b/intel/intel_chipset.h @@ -499,15 +499,6 @@ IS_GEMINILAKE(devid) || \ IS_COFFEELAKE(devid)) -#define IS_9XX(dev)(IS_GEN3(dev) || \ -IS_GEN4(dev) || \ -IS_GEN5(dev) || \ -IS_GEN6(dev) || \ -IS_GEN7(dev) || \ -IS_GEN8(dev) || \ -IS_GEN9(dev) || \ -IS_GEN10(dev)) - #define IS_CNL_Y(devid)((devid) == PCI_CHIP_CANNONLAKE_Y_GT2_0 || \ (devid) == PCI_CHIP_CANNONLAKE_Y_GT2_1 || \ (devid) == PCI_CHIP_CANNONLAKE_Y_GT2_2 || \ @@ -525,4 +516,13 @@ #define IS_GEN10(devid)(IS_CANNONLAKE(devid)) +#define IS_9XX(dev)(IS_GEN3(dev) || \ +IS_GEN4(dev) || \ +IS_GEN5(dev) || \ +IS_GEN6(dev) || \ +IS_GEN7(dev) || \ +IS_GEN8(dev) || \ +IS_GEN9(dev) || \ +IS_GEN10(dev)) + #endif /* _INTEL_CHIPSET_H */ -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 1/2] util: move some files to an ASIC neutral directory.
Change-Id: Iac1c4870253e8b8860a61b7cf175e7a25cc95921
Signed-off-by: Samuel Li
---
Makefile.sources | 10 +-
amdgpu/Makefile.am | 3 +-
amdgpu/Makefile.sources | 7 +-
amdgpu/amdgpu_asic_id.c | 219 ---
amdgpu/amdgpu_device.c | 7 +-
amdgpu/amdgpu_internal.h | 11 +-
amdgpu/util_hash.c | 387 ---
amdgpu/util_hash.h | 107 -
amdgpu/util_hash_table.c | 262
amdgpu/util_hash_table.h | 73 -
util/util_asic_id.c | 217 ++
util/util_asic_id.h | 39 +
util/util_hash.c | 387 +++
util/util_hash.h | 107 +
util/util_hash_table.c | 262
util/util_hash_table.h | 73 +
16 files changed, 1102 insertions(+), 1069 deletions(-)
delete mode 100644 amdgpu/amdgpu_asic_id.c
delete mode 100644 amdgpu/util_hash.c
delete mode 100644 amdgpu/util_hash.h
delete mode 100644 amdgpu/util_hash_table.c
delete mode 100644 amdgpu/util_hash_table.h
create mode 100644 util/util_asic_id.c
create mode 100644 util/util_asic_id.h
create mode 100644 util/util_hash.c
create mode 100644 util/util_hash.h
create mode 100644 util/util_hash_table.c
create mode 100644 util/util_hash_table.h
diff --git a/Makefile.sources b/Makefile.sources
index 10aa1d0..f2b0ec6 100644
--- a/Makefile.sources
+++ b/Makefile.sources
@@ -10,12 +10,18 @@ LIBDRM_FILES := \
libdrm_macros.h \
libdrm_lists.h \
util_double_list.h \
- util_math.h
+ util_math.h \
+ util/util_asic_id.c \
+ util/util_hash.c \
+ util/util_hash_table.c
LIBDRM_H_FILES := \
libsync.h \
xf86drm.h \
- xf86drmMode.h
+ xf86drmMode.h \
+ util/util_asic_id.h \
+ util/util_hash.h \
+ util/util_hash_table.h
LIBDRM_INCLUDE_H_FILES := \
include/drm/drm.h \
diff --git a/amdgpu/Makefile.am b/amdgpu/Makefile.am
index 66f6f67..c3e83d6 100644
--- a/amdgpu/Makefile.am
+++ b/amdgpu/Makefile.am
@@ -28,7 +28,8 @@ AM_CFLAGS = \
$(WARN_CFLAGS) \
-I$(top_srcdir) \
$(PTHREADSTUBS_CFLAGS) \
- -I$(top_srcdir)/include/drm
+ -I$(top_srcdir)/include/drm \
+ -I$(top_srcdir)/util
libdrmdatadir = @libdrmdatadir@
ASIC_ID_TABLE_NUM_ENTRIES := $(shell egrep -ci '^[0-9a-f]{4},.*[0-9a-f]+,' \
diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index bc3abaa..23e9e69 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -1,15 +1,10 @@
LIBDRM_AMDGPU_FILES := \
- amdgpu_asic_id.c \
amdgpu_bo.c \
amdgpu_cs.c \
amdgpu_device.c \
amdgpu_gpu_info.c \
amdgpu_internal.h \
- amdgpu_vamgr.c \
- util_hash.c \
- util_hash.h \
- util_hash_table.c \
- util_hash_table.h
+ amdgpu_vamgr.c
LIBDRM_AMDGPU_H_FILES := \
amdgpu.h
diff --git a/amdgpu/amdgpu_asic_id.c b/amdgpu/amdgpu_asic_id.c
deleted file mode 100644
index 3a88896..000
--- a/amdgpu/amdgpu_asic_id.c
+++ /dev/null
@@ -1,219 +0,0 @@
-/*
- * Copyright © 2017 Advanced Micro Devices, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "xf86drm.h"
-#include "amdgpu_drm.h"
-#include "amdgpu_internal.h"
-
-static int parse_one_line(const char *line, struct amdgpu_asic_id *id)
-{
- char *buf, *saveptr;
- char *s_did;
- char *s_rid;
- char *s_name;
- char *endptr;
- int r = 0;
-
- buf = strdup(line);
- if (!buf)
- return -ENOMEM;
-
- /* ignore empty line and commented line */
- if (strlen(line) == 0 || line[0] == '#') {
-
[Bug 101653] FAN speed is too high when displays are off
https://bugs.freedesktop.org/show_bug.cgi?id=101653 --- Comment #4 from Alexander Tsoy --- (In reply to Alex Deucher from comment #3) > Created attachment 132374 [details] [review] > possible fix It works. Thanks. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
