Re: [PATCH v2] drm/msm/dpu: Correct dpu destroy and disable order
On 11/2/2018 6:19 PM, Jayant Shekhar wrote: In case of msm drm bind failure, dpu_mdss_destroy is triggered. In this function, resources are freed and pm runtime disable is called, which triggers dpu_mdss_disable. Now in dpu_mdss_disable, driver tries to access a memory which is already freed. This results in kernel panic. Fix this by ensuring proper sequence of dpu destroy and disable calls. Changes in v2: - Removed double spacings [Jeykumar] Signed-off-by: Jayant Shekhar I was testing this patch out and in my setup I see that with this change, during a dsi probe defer case, a dpu_mdss_enable() is called followed by a dpu_mdss_destroy() but dpu_mdss_disable() is never called. This results in a mismatch in clock usecount causing the mdp clocks to stay enabled even with display turned off. localhost ~ # cat /sys/kernel/debug/clk/clk_summary | grep mdss disp_cc_mdss_rscc_ahb_clk000 0 0 0 5 disp_cc_mdss_axi_clk 000 0 0 0 5 disp_cc_mdss_ahb_clk 000 0 0 0 5 disp_cc_mdss_mdp_clk_src 1101920 0 0 5 disp_cc_mdss_mdp_lut_clk 0001920 0 0 5 disp_cc_mdss_mdp_clk1101920 0 0 5 disp_cc_mdss_vsync_clk_src 0001920 0 0 5 Also to note, on an older 4.14 kernel, I do not see any issue in the order in which these functions are called in the dsi probe defer case, i.e its an enable followed by a disable and then a destroy (and that's without this patch) --- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c index fd9c893..902bb4c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c @@ -156,18 +156,15 @@ static void dpu_mdss_destroy(struct drm_device *dev) struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss); struct dss_module_power *mp = &dpu_mdss->mp; + pm_runtime_disable(dev->dev); _dpu_mdss_irq_domain_fini(dpu_mdss); - free_irq(platform_get_irq(pdev, 0), dpu_mdss); - msm_dss_put_clk(mp->clk_config, mp->num_clk); devm_kfree(&pdev->dev, mp->clk_config); if (dpu_mdss->mmio) devm_iounmap(&pdev->dev, dpu_mdss->mmio); dpu_mdss->mmio = NULL; - - pm_runtime_disable(dev->dev); priv->mdss = NULL; } ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199025] Suspend hangs with Nouveau loaded. Never fully suspends and impossible to return to running state. - Asus PRIME Z270-P
https://bugzilla.kernel.org/show_bug.cgi?id=199025 Chen Yu (yu.c.c...@intel.com) changed: What|Removed |Added Component|Hibernation/Suspend |Video(Other) Product|Power Management|Drivers Summary|Suspend hangs. Never fully |Suspend hangs with Nouveau |suspends and impossible to |loaded. Never fully |return to running state. - |suspends and impossible to |Asus PRIME Z270-P |return to running state. - ||Asus PRIME Z270-P --- Comment #39 from Chen Yu (yu.c.c...@intel.com) --- @todd, is there any chance that you can test if the fc28 works for you? -- 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: [PATCH 2/2] drm: Revert syncobj timeline changes.
On 2018年11月09日 00:52, Christian König wrote:
Am 08.11.18 um 17:07 schrieb Koenig, Christian:
Am 08.11.18 um 17:04 schrieb Eric Anholt:
Daniel suggested I submit this, since we're still seeing regressions
from it. This is a revert to before 48197bc564c7 ("drm: add syncobj
timeline support v9") and its followon fixes.
This is a harmless false positive from lockdep, Chouming and I are
already working on a fix.
On the other hand we had enough trouble with that patch, so if it
really bothers you feel free to add my Acked-by: Christian König
and push it.
NAK, please no, I don't think this needed, the Warning totally isn't
related to syncobj timeline, but fence-array implementation flaw, just
exposed by syncobj.
In addition, Christian already has a fix for this Warning, I've tested.
Please Christian send to public review.
-David
Christian.
Christian.
Fixes this on first V3D testcase execution:
[ 48.767088]
[ 48.772410] WARNING: possible recursive locking detected
[ 48.39] 4.19.0-rc6+ #489 Not tainted
[ 48.781668]
[ 48.786993] shader_runner/3284 is trying to acquire lock:
[ 48.792408] ce309d7f (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.800714]
[ 48.800714] but task is already holding lock:
[ 48.806559] c5952bd3 (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.814862]
[ 48.814862] other info that might help us debug this:
[ 48.821410] Possible unsafe locking scenario:
[ 48.821410]
[ 48.827338] CPU0
[ 48.829788]
[ 48.832239] lock(&(&array->lock)->rlock);
[ 48.836434] lock(&(&array->lock)->rlock);
[ 48.840640]
[ 48.840640] *** DEADLOCK ***
[ 48.840640]
[ 48.846582] May be due to missing lock nesting notation
[ 130.763560] 1 lock held by cts-runner/3270:
[ 130.767745] #0: 7834b793 (&(&array->lock)->rlock){-...}, at:
dma_fence_add_callback+0x30/0x23c
[ 130.776461]
stack backtrace:
[ 130.780825] CPU: 1 PID: 3270 Comm: cts-runner Not tainted
4.19.0-rc6+ #486
[ 130.787706] Hardware name: Broadcom STB (Flattened Device Tree)
[ 130.793645] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 130.801404] [] (show_stack) from []
(dump_stack+0xa8/0xd4)
[ 130.808642] [] (dump_stack) from []
(__lock_acquire+0x848/0x1a68)
[ 130.816483] [] (__lock_acquire) from []
(lock_acquire+0xd8/0x22c)
[ 130.824326] [] (lock_acquire) from []
(_raw_spin_lock_irqsave+0x54/0x68)
[ 130.832777] [] (_raw_spin_lock_irqsave) from
[] (dma_fence_add_callback+0x30/0x23c)
[ 130.842183] [] (dma_fence_add_callback) from
[] (dma_fence_array_enable_signaling+0x58/0xec)
[ 130.852371] [] (dma_fence_array_enable_signaling) from
[] (dma_fence_add_callback+0xe8/0x23c)
[ 130.862647] [] (dma_fence_add_callback) from
[] (drm_syncobj_wait_ioctl+0x518/0x614)
[ 130.872143] [] (drm_syncobj_wait_ioctl) from
[] (drm_ioctl_kernel+0xb0/0xf0)
[ 130.880940] [] (drm_ioctl_kernel) from []
(drm_ioctl+0x1d8/0x390)
[ 130.888782] [] (drm_ioctl) from []
(do_vfs_ioctl+0xb0/0x8ac)
[ 130.896187] [] (do_vfs_ioctl) from []
(ksys_ioctl+0x34/0x60)
[ 130.903593] [] (ksys_ioctl) from []
(ret_fast_syscall+0x0/0x28)
Cc: Chunming Zhou
Cc: Christian König
Cc: Daniel Vetter
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/drm_syncobj.c | 359
+++---
include/drm/drm_syncobj.h | 73 ---
2 files changed, 105 insertions(+), 327 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index da8175d9c6ff..e2c5b3ca4824 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,9 +56,6 @@
#include "drm_internal.h"
#include
-/* merge normal syncobj to timeline syncobj, the point interval
is 1 */
-#define DRM_SYNCOBJ_BINARY_POINT 1
-
struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -74,29 +71,7 @@ static const struct dma_fence_ops
drm_syncobj_stub_fence_ops = {
.get_timeline_name = drm_syncobj_stub_fence_get_name,
};
-struct drm_syncobj_signal_pt {
- struct dma_fence_array *fence_array;
- u64 value;
- struct list_head list;
-};
-
-static DEFINE_SPINLOCK(signaled_fence_lock);
-static struct dma_fence signaled_fence;
-static struct dma_fence *drm_syncobj_get_stub_fence(void)
-{
- spin_lock(&signaled_fence_lock);
- if (!signaled_fence.ops) {
- dma_fence_init(&signaled_fence,
- &drm_syncobj_stub_fence_ops,
- &signaled_fence_lock,
- 0, 0);
- dma_fence_signal_locked(&signaled_fence);
- }
- spin_unlock(&signaled_fence_lock);
-
- return dma_fence_get(&signaled_fence);
-}
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -123,27 +98,6 @@ struct drm_syncobj *drm_syncobj_find(s
Re: [PATCH v4 1/3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file
Hi,
On Fri, Nov 2, 2018 at 3:26 PM Jeykumar Sankaran wrote:
>
> DPU is short for the Display Processing Unit. It is the display
> controller on Qualcomm SDM845 chips.
>
> This change adds MDSS and DSI nodes to enable display on the
> target device.
>
> Changes in v2:
> - Beefed up commit message
> - Use SoC specific compatibles for mdss and dpu (Rob H)
> - Use assigned-clocks to set initial clock frequency(Rob H)
> Changes in v3:
> - added IOMMU node
> - Fix device naming (remove _phys)
> - Use correct IRQ_TYPE in interrupt specifiers
> Changes in v4:
> - move mdss node to preserve the unit address sort order
> - remove _clk suffix from dsi clocks
> (both the comments are from Doug Anderson)
>
> Signed-off-by: Jeykumar Sankaran
> Signed-off-by: Sean Paul
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 191
> +++
> 1 file changed, 191 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index b72bdb0..5728b4c 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1248,6 +1248,197 @@
> };
> };
>
> + mdss: mdss@ae0 {
> + compatible = "qcom,sdm845-mdss";
> + reg = <0xae0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = <&dispcc 0>;
Could be done in a follow-up patch, but technically the above should be:
power-domains = <&dispcc MDSS_GDSC>;
> + dsi0: dsi@ae94000 {
> + compatible = "qcom,mdss-dsi-ctrl";
> + reg = <0xae94000 0x400>;
> + reg-names = "dsi_ctrl";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
Just out of curiousity, where does the 4 comes from? I guess this matches:
#define MDSS_HW_INTR_STATUS_INTR_DSI0 0x0010
#define MDSS_HW_INTR_STATUS_INTR_DSI1 0x0020
...where 0x10 == (1 << 4)
I don't see any bindings for this define so I guess hardcoding it is
fine (and the 5 for DSI1).
> + dsi0_phy: dsi-phy@ae94400 {
> + compatible = "qcom,dsi-phy-10nm";
> + reg = <0xae94400 0x200>,
> + <0xae94a00 0x1e0>,
> + <0xae94600 0x280>;
> + reg-names = "dsi_phy",
> + "dsi_pll",
> + "dsi_phy_lane";
> +
> + #clock-cells = <1>;
> + #phy-cells = <0>;
> +
> + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>;
> + clock-names = "iface";
> + };
+Matthias will probably want to base a future patch on this one since
he's trying to hook the clocks up more properly. ...but what you have
above matches the current bindings so I think we're good.
Overall: I am not massively familiar with display / bridge / panel
bindings but as far as I can tell this patch is good and ready to
land. Future work (like fixing the nit about using the power domain
#define) can always be done in a follow-up patch. Thus:
Reviewed-by: Douglas Anderson
I've tested this patch and it's helped me get a working display. Thus:
Tested-by: Douglas Anderson
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v2 1/2] drm/dp/mst: Reprobe EDID for MST ports on resume
Are you still looking into this at all? Even if it's not the right approach I'm still interested in seeing this get fixed as well/discussing what I said before On Wed, 2018-10-24 at 22:45 +, Li, Juston wrote: > On Wed, 2018-10-24 at 18:09 -0400, Lyude Paul wrote: > > Since there's going to be quite a number of changes I need to make to > > this I'm > > just going to make the changes myself! I'll make sure to Cc you with > > the > > respin > > Sounds good, thanks for picking it up! > > Juston > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Performance regression in ast drm driver
On Thu, Nov 8, 2018 at 10:05 PM Jean Delvare wrote: > > On Thu, 1 Nov 2018 16:27:07 +0100, Jean Delvare wrote: > > Hi David, > > > > The following commit: > > > > commit 7cf321d118a825c1541b43ca45294126fd474efa > > Author: Dave Airlie > > Date: Mon Oct 24 15:37:48 2016 +1000 > > > > drm/drivers: add support for using the arch wc mapping API. > > > > is causing a huge performance regression for the ast drm driver. In a > > text console, if I call "cat" on a large text file, it takes almost > > twice as much time to be displayed and scrolled completely. > > > > Can you please check that the ast driver portion of that commit is both > > correct and complete? > > And in the meantime, what bad will happen if we just revert the ast > portion of that commit? > This seems likely to be a hw problem with PCI writes to the AST "GPU", since it's just some sort of RAM + ARM on the end of a PCIE bus, we've definitely seen possible issues in the past with write combining around some of the mga GPUs with some CPUs. Have we seen the problem across a number of AST devices? Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 4/5] drm/nouveau: Use atomic VCPI helpers for MST
Currently, nouveau uses the yolo method of setting up MST displays: it
uses the old VCPI helpers (drm_dp_find_vcpi_slots()) for computing the
display configuration. These helpers don't take care to make sure they
take a reference to the mstb port that they're checking, and
additionally don't actually check whether or not the topology still has
enough bandwidth to provide the VCPI tokens required.
So, drop usage of the old helpers and move entirely over to the atomic
helpers.
Signed-off-by: Lyude Paul
Acked-by: Daniel Vetter
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 50 +
1 file changed, 43 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6cbbae3f438b..0469ef9e1a54 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -747,16 +747,22 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
- struct nv50_mstc *mstc = nv50_mstc(conn_state->connector);
+ struct drm_atomic_state *state = crtc_state->state;
+ struct drm_connector *connector = conn_state->connector;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
struct nv50_mstm *mstm = mstc->mstm;
- int bpp = conn_state->connector->display_info.bpc * 3;
+ int bpp = connector->display_info.bpc * 3;
int slots;
- mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, bpp);
-
- slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn);
- if (slots < 0)
- return slots;
+ mstc->pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock,
+bpp);
+ /* Zombies don't need VCPI */
+ if (!drm_connector_is_unregistered(connector)) {
+ slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
+ mstc->port, mstc->pbn);
+ if (slots < 0)
+ return slots;
+ }
return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
mstc->native);
@@ -920,12 +926,38 @@ nv50_mstc_get_modes(struct drm_connector *connector)
return ret;
}
+static int
+nv50_mstc_atomic_check(struct drm_connector *connector,
+ struct drm_connector_state *new_conn_state)
+{
+ struct drm_atomic_state *state = new_conn_state->state;
+ struct nv50_mstc *mstc = nv50_mstc(connector);
+ struct drm_dp_mst_topology_mgr *mgr = &mstc->mstm->mgr;
+ struct drm_connector_state *old_conn_state;
+ struct drm_crtc *old_crtc;
+
+ old_conn_state = drm_atomic_get_old_connector_state(state, connector);
+ old_crtc = old_conn_state->crtc;
+
+ /* We only need to release VCPI here if we're going from having a CRTC
+* on this connector, to not having one
+*/
+ if (!old_crtc || new_conn_state->crtc)
+ return 0;
+
+ /* Release the previous VCPI allocation since the encoder's
+* atomic_check() won't be called
+*/
+ return drm_dp_atomic_release_vcpi_slots(state, mgr, mstc->port);
+}
+
static const struct drm_connector_helper_funcs
nv50_mstc_help = {
.get_modes = nv50_mstc_get_modes,
.mode_valid = nv50_mstc_mode_valid,
.best_encoder = nv50_mstc_best_encoder,
.atomic_best_encoder = nv50_mstc_atomic_best_encoder,
+ .atomic_check = nv50_mstc_atomic_check,
};
static enum drm_connector_status
@@ -2109,6 +2141,10 @@ nv50_disp_atomic_check(struct drm_device *dev, struct
drm_atomic_state *state)
return ret;
}
+ ret = drm_dp_mst_atomic_check(state);
+ if (ret)
+ return ret;
+
return 0;
}
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/5] drm/dp_mst: Add some atomic state iterator macros
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
---
include/drm/drm_dp_mst_helper.h | 77 +
1 file changed, 77 insertions(+)
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 59f005b419cf..3faceb66f5cb 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -628,4 +628,81 @@ int drm_dp_atomic_release_vcpi_slots(struct
drm_atomic_state *state,
int drm_dp_send_power_updown_phy(struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_mst_port *port, bool power_up);
+extern const struct drm_private_state_funcs drm_dp_mst_topology_state_funcs;
+
+static inline bool
+__drm_dp_mst_state_iter_get(struct drm_atomic_state *state,
+ struct drm_dp_mst_topology_mgr **mgr,
+ struct drm_dp_mst_topology_state **old_state,
+ struct drm_dp_mst_topology_state **new_state,
+ int i)
+{
+ struct __drm_private_objs_state *objs_state = &state->private_objs[i];
+
+ if (objs_state->ptr->funcs != &drm_dp_mst_topology_state_funcs)
+ return false;
+
+ *mgr = to_dp_mst_topology_mgr(objs_state->ptr);
+ if (old_state)
+ *old_state = to_dp_mst_topology_state(objs_state->old_state);
+ if (new_state)
+ *new_state = to_dp_mst_topology_state(objs_state->new_state);
+
+ return true;
+}
+
+/**
+ * for_each_oldnew_mst_mgr_in_state - iterate over all DP MST topology
+ * managers in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking both old and new state. This is useful in places where the state
+ * delta needs to be considered, for example in atomic check functions.
+ */
+#define for_each_oldnew_mst_mgr_in_state(__state, mgr, old_state, new_state,
__i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
&(old_state), &(new_state), (__i)))
+
+/**
+ * for_each_old_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @old_state: &struct drm_dp_mst_topology_state iteration cursor for the old
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking only the old state. This is useful in disable functions, where we
+ * need the old state the hardware is still in.
+ */
+#define for_each_old_mst_mgr_in_state(__state, mgr, old_state, __i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
&(old_state), NULL, (__i)))
+
+/**
+ * for_each_new_mst_mgr_in_state - iterate over all DP MST topology managers
+ * in an atomic update
+ * @__state: &struct drm_atomic_state pointer
+ * @mgr: &struct drm_dp_mst_topology_mgr iteration cursor
+ * @new_state: &struct drm_dp_mst_topology_state iteration cursor for the new
+ * state
+ * @__i: int iteration cursor, for macro-internal use
+ *
+ * This iterates over all DRM DP MST topology managers in an atomic update,
+ * tracking only the new state. This is useful in enable functions, where we
+ * need the new state the hardware should be in when the atomic commit
+ * operation has completed.
+ */
+#define for_each_new_mst_mgr_in_state(__state, mgr, new_state, __i) \
+ for ((__i) = 0; (__i) < (__state)->num_private_objs; (__i)++) \
+ for_each_if(__drm_dp_mst_state_iter_get((__state), &(mgr),
NULL, &(new_state), (__i)))
+
#endif
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/5] drm/dp_mst: Improve VCPI helpers, use in nouveau
This patchset does some cleaning up of the atomic VCPI helpers for MST, and converts nouveau over to using them. I would have included amdgpu in this patch as well, but at the moment moving them over to the atomic helpers is nontrivial. Cc: Daniel Vetter Lyude Paul (5): drm/dp_mst: Add some atomic state iterator macros drm/dp_mst: Start tracking per-port VCPI allocations drm/dp_mst: Check payload count in drm_dp_mst_atomic_check() drm/nouveau: Use atomic VCPI helpers for MST drm/dp_mst: Stop unsetting mstc->port, check connector registration drivers/gpu/drm/drm_dp_mst_topology.c | 251 drivers/gpu/drm/i915/intel_display.c| 4 + drivers/gpu/drm/i915/intel_dp_mst.c | 31 +-- drivers/gpu/drm/nouveau/dispnv50/disp.c | 65 -- include/drm/drm_dp_mst_helper.h | 101 +- 5 files changed, 382 insertions(+), 70 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/5] drm/dp_mst: Check payload count in drm_dp_mst_atomic_check()
It occurred to me that we never actually check this! So let's start
doing that.
Signed-off-by: Lyude Paul
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/drm_dp_mst_topology.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 42443275624b..89a6d48314ef 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3222,7 +3222,7 @@ drm_dp_mst_atomic_check_topology_state(struct
drm_dp_mst_topology_mgr *mgr,
{
struct drm_dp_vcpi_allocation *vcpi;
struct drm_dp_mst_port *port;
- int avail_slots = 63, ret;
+ int avail_slots = 63, payload_count = 0, ret;
/* There's no possible scenario where releasing VCPI or keeping it the
* same would make the state invalid
@@ -3260,6 +3260,13 @@ drm_dp_mst_atomic_check_topology_state(struct
drm_dp_mst_topology_mgr *mgr,
goto port_fail;
}
+ if (++payload_count > mgr->max_payloads) {
+ DRM_DEBUG_ATOMIC("[MST MGR:%p] state %p has too many
payloads (max=%d)\n",
+mgr, mst_state, mgr->max_payloads);
+ ret = -EINVAL;
+ goto port_fail;
+ }
+
drm_dp_put_port(port);
}
DRM_DEBUG_ATOMIC("[MST MGR:%p] mst state %p VCPI avail=%d used=%d\n",
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 5/5] drm/dp_mst: Stop unsetting mstc->port, check connector registration
Same thing we did in i915, but for nouveau now.
Signed-off-by: Lyude Paul
Cc: Daniel Vetter
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 0469ef9e1a54..ff04a56b5949 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -698,8 +698,10 @@ nv50_msto_cleanup(struct nv50_msto *msto)
struct nv50_mstm *mstm = mstc->mstm;
NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
- if (mstc->port && mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
+ if (!drm_connector_is_unregistered(&mstc->connector) &&
+ mstc->port->vcpi.vcpi > 0 && !nv50_msto_payload(msto))
drm_dp_mst_deallocate_vcpi(&mstm->mgr, mstc->port);
+
if (msto->disabled) {
msto->mstc = NULL;
msto->head = NULL;
@@ -725,7 +727,8 @@ nv50_msto_prepare(struct nv50_msto *msto)
};
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
- if (mstc->port && mstc->port->vcpi.vcpi > 0) {
+ if (!drm_connector_is_unregistered(&mstc->connector) &&
+ mstc->port->vcpi.vcpi > 0) {
struct drm_dp_payload *payload = nv50_msto_payload(msto);
if (payload) {
args.vcpi.start_slot = payload->start_slot;
@@ -828,7 +831,7 @@ nv50_msto_disable(struct drm_encoder *encoder)
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
- if (mstc->port)
+ if (!drm_connector_is_unregistered(&mstc->connector))
drm_dp_mst_reset_vcpi_slots(&mstm->mgr, mstc->port);
mstm->outp->update(mstm->outp, msto->head->base.index, NULL, 0, 0);
@@ -967,7 +970,7 @@ nv50_mstc_detect(struct drm_connector *connector, bool
force)
enum drm_connector_status conn_status;
int ret;
- if (!mstc->port)
+ if (drm_connector_is_unregistered(&mstc->connector))
return connector_status_disconnected;
ret = pm_runtime_get_sync(connector->dev->dev);
@@ -1105,10 +1108,6 @@ nv50_mstm_destroy_connector(struct
drm_dp_mst_topology_mgr *mgr,
drm_fb_helper_remove_one_connector(&drm->fbcon->helper,
&mstc->connector);
- drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
- mstc->port = NULL;
- drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
-
drm_connector_put(&mstc->connector);
}
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/5] drm/dp_mst: Start tracking per-port VCPI allocations
There has been a TODO waiting for quite a long time in drm_dp_mst_topology.c: /* We cannot rely on port->vcpi.num_slots to update * topology_state->avail_slots as the port may not exist if the parent * branch device was unplugged. This should be fixed by tracking * per-port slot allocation in drm_dp_mst_topology_state instead of * depending on the caller to tell us how many slots to release. */ That's not the only reason we should fix this: forcing the driver to track the VCPI allocations throughout a state's atomic check is error prone, because it means that extra care has to be taken with the order that drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() are called in in order to ensure idempotency. Currently the only driver actually using these helpers, i915, doesn't even do this correctly: multiple ->best_encoder() checks with i915's current implementation would not be idempotent and would over-allocate VCPI slots, something I learned trying to implement fallback retraining in MST. So: simplify this whole mess, and teach drm_dp_atomic_find_vcpi_slots() and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations for each port. This allows us to ensure idempotency without having to rely on the driver as much. Additionally: the driver doesn't need to do any kind of VCPI slot tracking anymore if it doesn't need it for it's own internal state. Additionally; this adds a new drm_dp_mst_atomic_check() helper which must be used by atomic drivers to perform validity checks for the new VCPI allocations incurred by a state. Also: update the documentation and make it more obvious that these /must/ be called by /all/ atomic drivers supporting MST. Changes since v4: - Don't skip the atomic checks for VCPI allocations if no new VCPI allocations happen in a state. This makes the next change I'm about to list here a lot easier to implement. - Don't ignore VCPI allocations on destroyed ports, instead ensure that when ports are destroyed and still have VCPI allocations in the topology state, the only state changes allowed are releasing said ports' VCPI. This prevents a state with a mix of VCPI allocations from destroyed ports, and allocations from valid ports. Changes since v3: - Don't release VCPI allocations in the topology state immediately in drm_dp_atomic_release_vcpi_slots(), instead mark them as 0 and skip over them in drm_dp_mst_duplicate_state(). This makes it so drm_dp_atomic_release_vcpi_slots() is still idempotent while also throwing warnings if the driver messes up it's book keeping and tries to release VCPI slots on a port that doesn't have any pre-existing VCPI allocation - danvet - Change mst_state/state in some debugging messages to "mst state" Changes since v2: - Use kmemdup() for duplicating MST state - danvet - Move port validation out of duplicate state callback - danvet - Handle looping through MST topology states in drm_dp_mst_atomic_check() so the driver doesn't have to do it - Fix documentation in drm_dp_atomic_find_vcpi_slots() - Move the atomic check for each individual topology state into it's own function, reduces indenting - Don't consider "stale" MST ports when calculating the bandwidth requirements. This is needed because originally we relied on the state duplication functions to prune any stale ports from the new state, which would prevent us from incorrectly considering their bandwidth requirements alongside legitimate new payloads. - Add function references in drm_dp_atomic_release_vcpi_slots() - danvet - Annotate atomic VCPI and atomic check functions with __must_check - danvet Changes since v1: - Don't use the now-removed ->atomic_check() for private objects hook, just give drivers a function to call themselves Signed-off-by: Lyude Paul Cc: Daniel Vetter --- drivers/gpu/drm/drm_dp_mst_topology.c | 244 ++ drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_dp_mst.c | 31 ++-- include/drm/drm_dp_mst_helper.h | 24 ++- 4 files changed, 248 insertions(+), 55 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 8c3cfac437f4..42443275624b 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2614,21 +2614,34 @@ static int drm_dp_init_vcpi(struct drm_dp_mst_topology_mgr *mgr, } /** - * drm_dp_atomic_find_vcpi_slots() - Find and add vcpi slots to the state + * drm_dp_atomic_find_vcpi_slots() - Find and add VCPI slots to the state * @state: global atomic state * @mgr: MST topology manager for the port * @port: port to find vcpi slots for * @pbn: bandwidth required for the mode in PBN * - * RETURNS: - * Total slots in the atomic state assigned for this port or error + * Allocates VCPI slots to @port, replacing any previous VCPI allocations it + * may have had
Re: [PATCH v2 1/3] drm/msm: dpu: Mask inactive pending flushes
On 2018-11-08 13:40, Sean Paul wrote:
On Thu, Nov 08, 2018 at 01:03:03PM -0800, Jeykumar Sankaran wrote:
On 2018-10-30 09:00, Sean Paul wrote:
> From: Sean Paul
>
> This patch masks any pending flushes which have not been latched for a
> commit. This will catch the case where an asynchronous update is
> nullified by a disable in the same frame.
>
> Changes in v2:
> - Added to the set
>
> Signed-off-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 8fa601a9abbf..d7a7fedc09f7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -28,6 +28,7 @@
> #define CTL_TOP 0x014
> #define CTL_FLUSH 0x018
> #define CTL_START 0x01C
> +#define CTL_FLUSH_MASK0x090
> #define CTL_PREPARE 0x0d0
> #define CTL_SW_RESET 0x030
> #define CTL_LAYER_EXTN_OFFSET 0x40
> @@ -121,6 +122,12 @@ static inline void
dpu_hw_ctl_trigger_flush(struct
> dpu_hw_ctl *ctx)
> {
>trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
> dpu_hw_ctl_get_flush_register(ctx));
> +
> + /*
> + * Async updates could have changed CTL_FLUSH since it was last
> latched.
> + * Mask anything not involved in this latest commit.
> + */
> + DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
Do we need this change for adding the current async cursor support?
Hmm, I think you asked me to implement this at the weekly meeting a
little
while ago. Apparently HW team requested that we mask off the bits for
planes which have been disabled-but-not-flushed?
OK. If you want to implement the HW team recommendation, you should
block the
FLUSH writes until both FLUSH and FLUSH_MASK writes goes through. We can
do
that by writing 0x to the FLUSH_MASK indicating "hardware is not
ready"
at the beginnging of the new vsync window. Since async updates dont wait
for commit_done (vsync), we can do that only for sync commits. Once we
are
done programming all the registers and the final flush bits are ready,
the
order of writing has to be reversed by writing FLUSH first and then
FLUSH_MASK
to the inverse of FLUSH to unblock the hardware programming on vsync.
Still, there is a small window of error where vsync can happen between
FLUSH
and FLUSH_MASK writes where we will end up missing the vsync but no
partial
frame registers will be programmed.
I believe we have decided to try out this approach with a fresh set of
patches
and let the current cursor support get in as such. In that case, we can
drop
this patch from this series.
Thanks,
Jeykumar S.
Sean
We are not masking any bit by default. So there is no need for
updating
it
here.
The usage of flush_mask is not completely explored yet. Maybe we can
add
this register support when we revisit this async logic as we
discussed.
Thanks and Regards,
Jeykumar S.
>DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> }
--
Jeykumar S
--
Jeykumar S
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] Allow fd.o to join forces with X.Org
The leadership of freedesktop.org (fd.o) has recently expressed interest
in having an elected governing body. Given the tight connection between
fd.o and X.Org and the fact that X.Org has such a governing body it
seemed obvious to consider extending X.Org's mandate to fd.o.
Quite a bit of background on fd.o leading up to this has been covered by
Daniel Stone at XDC 2018 [2] and was covered really well by Jake Edge of
LWN [1].
One question that is briefly addressed in the LWN article and was
thoroughly discussed by members of the X.Org boards, Daniel Stone, and
others in hallway discussions is the question of whether to extend the
X.Org membership to projects hosted on fd.o but outside the purpose of
the X.Org foundation as enacted in its bylaws.
Most people I talked to would prefer not to dilute X.Org's mission and
extend membership only to contributors of projects that follow X.Org's
purpose as enacted in its bylaws. Other projects can continue to be
hosted on fd.o but won't receive X.Org membership for the mere reason of
being hosted on fd.o.
[1] https://lwn.net/Articles/767258/
[2] https://youtu.be/s22B3E7rUTs
v3:
- Clarify what support of fd.o projects entails without formalizing a
two-tier system for fd.o projects that fall under X.Org's mandate and
those who don't
- Add link to Daniel's talk at XDC2018
v2:
- Subject line that better describes the intention
- Briefly describe reasons behind this change
- Drop expanding membership eligibility
Acked-by: Daniel Stone
---
We're looking for feedback and comments on this patch. If it's not
widely controversial the final version of the patch will be put to a
vote at the 2019 X.Org elections.
The patch applies to the X.Org bylaws git repo, which can be found at
https://gitlab.freedesktop.org/xorgfoundation/bylaws
Happy commenting.
Harry
bylaws.tex | 5 +
1 file changed, 5 insertions(+)
diff --git a/bylaws.tex b/bylaws.tex
index 4ab35a4f7745..5a7542739582 100644
--- a/bylaws.tex
+++ b/bylaws.tex
@@ -24,6 +24,11 @@ The purpose of the X.Org Foundation shall be to:
\item Support and educate the general community of users of this
graphics stack.
+
+ \item Support free and open source projects through the freedesktop.org
+ infrastructure. This includes, but is not limited to: Administering and
+ providing project hosting services.
+
\end{enumerate}
\article{INTERPRETATION}
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108649] On Vega GPU Project CARS 2 Demo cause broke fonts in gnome-shell
https://bugs.freedesktop.org/show_bug.cgi?id=108649 --- Comment #4 from Hin-Tak Leung --- Sorry for the noise - the webkit problem I mentioned earlier turned out to an a compositing problem of mesa; a fix went into mesa and upgrading mesa fixed it. The gnome extension setting panel missing text issue I haven't figured out yet. And I may still be affected by this as this have younger versions of other stuff I am using - so I am keeping an eye on this. -- 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: [Freedreno] [PATCH 1/2] drm/msm: use common display thread for dispatching vblank events
On 2018-11-01 12:09, Sean Paul wrote:
On Wed, Oct 31, 2018 at 05:19:04PM -0700, Jeykumar Sankaran wrote:
DPU was using one thread per display to dispatch async
commits and vblank requests. Since clean up already happened
in msm to use the common thread for all the display commits,
display threads are only used to cater vblank requests. Single
thread is sufficient to do the job without any performance hits.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 6 +---
drivers/gpu/drm/msm/msm_drv.c | 50
-
drivers/gpu/drm/msm/msm_drv.h | 2 +-
3 files changed, 23 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55ef..aff20f5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -753,11 +753,7 @@ static int dpu_encoder_resource_control(struct
drm_encoder *drm_enc,
is_vid_mode = dpu_enc->disp_info.capabilities &
MSM_DISPLAY_CAP_VID_MODE;
- if (drm_enc->crtc->index >= ARRAY_SIZE(priv->disp_thread)) {
- DPU_ERROR("invalid crtc index\n");
- return -EINVAL;
- }
- disp_thread = &priv->disp_thread[drm_enc->crtc->index];
+ disp_thread = &priv->disp_thread;
/*
* when idle_pc is not supported, process only KICKOFF, STOP and
MODESET
diff --git a/drivers/gpu/drm/msm/msm_drv.c
b/drivers/gpu/drm/msm/msm_drv.c
index 9c9f7ff..1f384b3 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -257,8 +257,7 @@ static int vblank_ctrl_queue_work(struct
msm_drm_private *priv,
list_add_tail(&vbl_ev->node, &vbl_ctrl->event_list);
spin_unlock_irqrestore(&vbl_ctrl->lock, flags);
- kthread_queue_work(&priv->disp_thread[crtc_id].worker,
- &vbl_ctrl->work);
+ kthread_queue_work(&priv->disp_thread.worker, &vbl_ctrl->work);
return 0;
}
@@ -284,14 +283,12 @@ static int msm_drm_uninit(struct device *dev)
kfree(vbl_ev);
}
+ kthread_flush_worker(&priv->disp_thread.worker);
+ kthread_stop(priv->disp_thread.thread);
I realize this is moving existing code, but is there a race here? You
can't have
work enqueued in between the flush and stop?
I looked further into this comment. Ideally, we call into msm_unbind
only when
the device is released and we release the device only on the last close
of
the drm device. So the userspace doesn't have any device handle to make
ioctl calls, which could queue jobs to this queue. Since we are making
sure
to flush out the last job already on the queue, we can safely call the
kthread_stop here.
Thanks and Regards,
Jeykumar S.
You might also want to use kthread_destroy_worker to do this work (in a
follow-up patch including the event threads too).
+ priv->disp_thread.thread = NULL;
+
/* clean up display commit/event worker threads */
This comment needs updating now
for (i = 0; i < priv->num_crtcs; i++) {
- if (priv->disp_thread[i].thread) {
-
kthread_flush_worker(&priv->disp_thread[i].worker);
- kthread_stop(priv->disp_thread[i].thread);
- priv->disp_thread[i].thread = NULL;
- }
-
if (priv->event_thread[i].thread) {
kthread_flush_worker(&priv->event_thread[i].worker);
kthread_stop(priv->event_thread[i].thread);
@@ -537,6 +534,22 @@ static int msm_drm_init(struct device *dev,
struct
drm_driver *drv)
ddev->mode_config.funcs = &mode_config_funcs;
ddev->mode_config.helper_private = &mode_config_helper_funcs;
+ /* initialize display thread */
+ kthread_init_worker(&priv->disp_thread.worker);
+ priv->disp_thread.dev = ddev;
+ priv->disp_thread.thread = kthread_run(kthread_worker_fn,
+ &priv->disp_thread.worker,
+ "disp_thread");
+ if (IS_ERR(priv->disp_thread.thread)) {
+ DRM_DEV_ERROR(dev, "failed to create crtc_commit
kthread\n");
+ priv->disp_thread.thread = NULL;
+ goto err_msm_uninit;
+ }
+
+ ret = sched_setscheduler(priv->disp_thread.thread, SCHED_FIFO,
¶m);
+ if (ret)
+ pr_warn("display thread priority update failed: %d\n",
ret);
+
/**
* this priority was found during empiric testing to have
appropriate
* realtime scheduling to process display updates and interact
with
@@ -544,27 +557,6 @@ static int msm_drm_init(struct device *dev,
struct
drm_driver *drv)
*/
param.sched_priority = 16;
for (i = 0; i < priv->num_crtcs; i++) {
-
- /* initialize display thread */
- priv->d
Re: [PATCH 2/2] drm/msm/dsi: Get PHY ref clock from the DT
Hi,
On Fri, Nov 2, 2018 at 2:45 PM Matthias Kaehlcke wrote:
>
> Get the PHY ref clock from the device tree instead of hardcoding
> its name and rate.
>
> Signed-off-by: Matthias Kaehlcke
> ---
> drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> index 4c03f0b7343ed..1016eb50df8f5 100644
> --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
> @@ -91,6 +91,8 @@ struct dsi_pll_10nm {
> void __iomem *phy_cmn_mmio;
> void __iomem *mmio;
>
> + struct clk *vco_ref_clk;
> +
> u64 vco_ref_clk_rate;
> u64 vco_current_rate;
>
> @@ -630,7 +632,8 @@ static int pll_10nm_register(struct dsi_pll_10nm
> *pll_10nm)
> char clk_name[32], parent[32], vco_name[32];
> char parent2[32], parent3[32], parent4[32];
> struct clk_init_data vco_init = {
> - .parent_names = (const char *[]){ "xo" },
> + .parent_names = (const char *[]){
> + __clk_get_name(pll_10nm->vco_ref_clk) },
> .num_parents = 1,
> .name = vco_name,
> .flags = CLK_IGNORE_UNUSED,
> @@ -786,6 +789,12 @@ struct msm_dsi_pll *msm_dsi_pll_10nm_init(struct
> platform_device *pdev, int id)
> pll_10nm->id = id;
> pll_10nm_list[id] = pll_10nm;
>
> + pll_10nm->vco_ref_clk = devm_clk_get(&pdev->dev, "ref");
> + if (IS_ERR(pll_10nm->vco_ref_clk)) {
> + dev_err(&pdev->dev, "couldn't get 'ref' clock\n");
> + return (void *)pll_10nm->vco_ref_clk;
> + }
So, u. Can you follow the same pattern for all the other clocks
in this file too? All parents should get their name based on
references in the device tree.
It turns out that right now we have a mismatch because
"drivers/clk/qcom/dispcc-sdm845.c" calls "dsi0pllbyte"
"dsi0_phy_pll_out_byteclk" and calls "dsi0pll"
"dsi0_phy_pll_out_dsiclk". We might want to change the names in
dispcc-sdm845.c, but it wouldn't matter if we simply didn't hardcode
them here.
-Doug
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/msm: dpu: Make legacy cursor updates asynchronous
On 2018-10-30 09:00, Sean Paul wrote:
From: Sean Paul
This patch sprinkles a few async/legacy_cursor_update checks
through commit to ensure that cursor updates aren't blocked on vsync.
There are 2 main components to this, the first is that we don't want to
wait_for_commit_done in msm_atomic before returning from
atomic_complete.
The second is that in dpu we don't want to wait for frame_done events
when
updating the cursor.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 44 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-
drivers/gpu/drm/msm/msm_atomic.c| 3 +-
6 files changed, 49 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index ed84cf44a222..1e3e57817b72 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -702,7 +702,7 @@ static int _dpu_crtc_wait_for_frame_done(struct
drm_crtc *crtc)
return rc;
}
-void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
+void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async)
{
struct drm_encoder *encoder;
struct drm_device *dev = crtc->dev;
@@ -731,27 +731,30 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
* Encoder will flush/start now, unless it has a tx
pending.
* If so, it may delay and flush at an irq event (e.g.
ppdone)
*/
- dpu_encoder_prepare_for_kickoff(encoder, ¶ms);
+ dpu_encoder_prepare_for_kickoff(encoder, ¶ms, async);
}
- /* wait for frame_event_done completion */
- DPU_ATRACE_BEGIN("wait_for_frame_done_event");
- ret = _dpu_crtc_wait_for_frame_done(crtc);
- DPU_ATRACE_END("wait_for_frame_done_event");
- if (ret) {
- DPU_ERROR("crtc%d wait for frame done
failed;frame_pending%d\n",
- crtc->base.id,
- atomic_read(&dpu_crtc->frame_pending));
- goto end;
- }
- if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
- /* acquire bandwidth and other resources */
- DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
- } else
- DPU_DEBUG("crtc%d commit\n", crtc->base.id);
+ if (!async) {
+ /* wait for frame_event_done completion */
+ DPU_ATRACE_BEGIN("wait_for_frame_done_event");
+ ret = _dpu_crtc_wait_for_frame_done(crtc);
+ DPU_ATRACE_END("wait_for_frame_done_event");
+ if (ret) {
+ DPU_ERROR("crtc%d wait for frame done
failed;frame_pending%d\n",
+ crtc->base.id,
+
atomic_read(&dpu_crtc->frame_pending));
+ goto end;
+ }
+
+ if (atomic_inc_return(&dpu_crtc->frame_pending) == 1) {
+ /* acquire bandwidth and other resources */
+ DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
+ } else
+ DPU_DEBUG("crtc%d commit\n", crtc->base.id);
- dpu_crtc->play_count++;
+ dpu_crtc->play_count++;
+ }
dpu_vbif_clear_errors(dpu_kms);
@@ -759,11 +762,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
if (encoder->crtc != crtc)
continue;
- dpu_encoder_kickoff(encoder);
+ dpu_encoder_kickoff(encoder, async);
}
end:
- reinit_completion(&dpu_crtc->frame_done_comp);
+ if (!async)
+ reinit_completion(&dpu_crtc->frame_done_comp);
DPU_ATRACE_END("crtc_commit");
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 4822602402f9..ec633ce3ee6c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -277,8 +277,9 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool
en);
/**
* dpu_crtc_commit_kickoff - trigger kickoff of the commit for this
crtc
* @crtc: Pointer to drm crtc object
+ * @async: true if the commit is asynchronous, false otherwise
*/
-void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
+void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async);
/**
* dpu_crtc_complete_commit - callback signalling completion of
current
commit
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 82c55efb500f..a8ba10ceaacf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1375,7 +1375,8 @@ static void dpu_encoder_off_work(struct
kthread_work
*wor
Re: [PATCH v2 1/3] drm/msm: dpu: Mask inactive pending flushes
On Thu, Nov 08, 2018 at 01:03:03PM -0800, Jeykumar Sankaran wrote:
> On 2018-10-30 09:00, Sean Paul wrote:
> > From: Sean Paul
> >
> > This patch masks any pending flushes which have not been latched for a
> > commit. This will catch the case where an asynchronous update is
> > nullified by a disable in the same frame.
> >
> > Changes in v2:
> > - Added to the set
> >
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > index 8fa601a9abbf..d7a7fedc09f7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > @@ -28,6 +28,7 @@
> > #define CTL_TOP 0x014
> > #define CTL_FLUSH 0x018
> > #define CTL_START 0x01C
> > +#define CTL_FLUSH_MASK0x090
> > #define CTL_PREPARE 0x0d0
> > #define CTL_SW_RESET 0x030
> > #define CTL_LAYER_EXTN_OFFSET 0x40
> > @@ -121,6 +122,12 @@ static inline void dpu_hw_ctl_trigger_flush(struct
> > dpu_hw_ctl *ctx)
> > {
> > trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
> > dpu_hw_ctl_get_flush_register(ctx));
> > +
> > + /*
> > +* Async updates could have changed CTL_FLUSH since it was last
> > latched.
> > +* Mask anything not involved in this latest commit.
> > +*/
> > + DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
> Do we need this change for adding the current async cursor support?
Hmm, I think you asked me to implement this at the weekly meeting a little
while ago. Apparently HW team requested that we mask off the bits for
planes which have been disabled-but-not-flushed?
Sean
> We are not masking any bit by default. So there is no need for updating it
> here.
>
> The usage of flush_mask is not completely explored yet. Maybe we can add
> this register support when we revisit this async logic as we discussed.
>
> Thanks and Regards,
> Jeykumar S.
> > DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> > }
>
> --
> Jeykumar S
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/edid: Add and export function to parse manufacturer id
On Thu, Nov 08, 2018 at 08:42:52PM +, Souza, Jose wrote:
> On Thu, 2018-11-08 at 09:31 +0100, Daniel Vetter wrote:
> > On Wed, Nov 07, 2018 at 04:23:52PM -0800, José Roberto de Souza
> > wrote:
> > > This function will be helpful to drivers that wants to add its own
> > > quirks to sinks.
> >
> > Why would you want to do that? The point of a shared edid parsing
> > code is
> > that we can share all these quirks ...
> >
> > For these kind of patches, always include the driver code that makes
> > use
> > of your new code too. That makes it much easier to answer these
> > questions.
>
> This will be used to disable or enable with quirks PSR in some panels
> that do not behave like eDP spec states.
> As this would be specifc to i915, I guess is better keep the list only
> in i915.
>
> What is your opinion about that?
For anything dp, shouldn't we use the OUI instead? Or is that more garbage
than the EDID serial?
And yes, psr quirking for i915 seems like a reasonable thing to do, using
either approach. But definitely include the full picture, including i915
patches, in your next round.
-Daniel
>
> >
> > Thanks, Daniel
> >
> >
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/drm_edid.c | 20
> > > include/drm/drm_edid.h | 1 +
> > > 2 files changed, 17 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c
> > > b/drivers/gpu/drm/drm_edid.c
> > > index b506e3622b08..1a0ddf3d326b 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -1755,6 +1755,21 @@ EXPORT_SYMBOL(drm_edid_duplicate);
> > >
> > > /*** EDID parsing ***/
> > >
> > > +/**
> > > + * drm_edid_manufacturer_parse - parse the EDID manufacturer id to
> > > readable
> > > + * characters and set into manufacturer parameter.
> > > + * @edid: EDID to get the manufacturer
> > > + * @manufacturer: the char buffer to store the id
> > > + */
> > > +void drm_edid_manufacturer_parse(const struct edid *edid, char
> > > manufacturer[3])
> > > +{
> > > + manufacturer[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > > + manufacturer[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > + ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> > > + manufacturer[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > +}
> > > +EXPORT_SYMBOL(drm_edid_manufacturer_parse);
> > > +
> > > /**
> > > * edid_vendor - match a string against EDID's obfuscated vendor
> > > field
> > > * @edid: EDID to match
> > > @@ -1766,10 +1781,7 @@ static bool edid_vendor(const struct edid
> > > *edid, const char *vendor)
> > > {
> > > char edid_vendor[3];
> > >
> > > - edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > > - edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > - ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> > > - edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > + drm_edid_manufacturer_parse(edid, edid_vendor);
> > >
> > > return !strncmp(edid_vendor, vendor, 3);
> > > }
> > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > > index e3c404833115..e4f3f7f34d6a 100644
> > > --- a/include/drm/drm_edid.h
> > > +++ b/include/drm/drm_edid.h
> > > @@ -466,6 +466,7 @@ struct edid *drm_get_edid_switcheroo(struct
> > > drm_connector *connector,
> > >struct i2c_adapter *adapter);
> > > struct edid *drm_edid_duplicate(const struct edid *edid);
> > > int drm_add_edid_modes(struct drm_connector *connector, struct
> > > edid *edid);
> > > +void drm_edid_manufacturer_parse(const struct edid *edid, char
> > > manufacturer[3]);
> > >
> > > u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
> > > enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8
> > > video_code);
> > > --
> > > 2.19.1
> > >
> > > ___
> > > 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 1/3] drm/msm: dpu: Mask inactive pending flushes
On 2018-10-30 09:00, Sean Paul wrote:
From: Sean Paul
This patch masks any pending flushes which have not been latched for a
commit. This will catch the case where an asynchronous update is
nullified by a disable in the same frame.
Changes in v2:
- Added to the set
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 8fa601a9abbf..d7a7fedc09f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -28,6 +28,7 @@
#define CTL_TOP 0x014
#define CTL_FLUSH 0x018
#define CTL_START 0x01C
+#define CTL_FLUSH_MASK0x090
#define CTL_PREPARE 0x0d0
#define CTL_SW_RESET 0x030
#define CTL_LAYER_EXTN_OFFSET 0x40
@@ -121,6 +122,12 @@ static inline void dpu_hw_ctl_trigger_flush(struct
dpu_hw_ctl *ctx)
{
trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
dpu_hw_ctl_get_flush_register(ctx));
+
+ /*
+* Async updates could have changed CTL_FLUSH since it was last
latched.
+* Mask anything not involved in this latest commit.
+*/
+ DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
Do we need this change for adding the current async cursor support?
We are not masking any bit by default. So there is no need for updating
it here.
The usage of flush_mask is not completely explored yet. Maybe we can add
this register support when we revisit this async logic as we discussed.
Thanks and Regards,
Jeykumar S.
DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
}
--
Jeykumar S
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/3] drm/msm: dpu: Only check flush register against pending flushes
On 2018-10-30 09:00, Sean Paul wrote: From: Sean Paul There exists a case where a flush of a plane/dma may have been triggered & started from an async commit. If that plane/dma is subsequently disabled by the next commit, the flush register will continue to hold the flush bit for the disabled plane. Since the bit remains active, pending_kickoff_cnt will never decrement and we'll miss frame_done events. This patch limits the check of flush_register to include only those bits which have been updated with the latest commit. Changes in v2: - None Reviewed-by: Jeykumar Sankaran Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index b3c68c4fcc8e..667f304c92ea 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -331,7 +331,7 @@ static void dpu_encoder_phys_vid_vblank_irq(void *arg, int irq_idx) if (hw_ctl && hw_ctl->ops.get_flush_register) flush_register = hw_ctl->ops.get_flush_register(hw_ctl); - if (flush_register == 0) + if (!(flush_register & hw_ctl->ops.get_pending_flush(hw_ctl))) new_cnt = atomic_add_unless(&phys_enc->pending_kickoff_cnt, -1, 0); spin_unlock_irqrestore(phys_enc->enc_spinlock, lock_flags); -- Jeykumar S ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/edid: Add and export function to parse manufacturer id
On Thu, 2018-11-08 at 09:31 +0100, Daniel Vetter wrote:
> On Wed, Nov 07, 2018 at 04:23:52PM -0800, José Roberto de Souza
> wrote:
> > This function will be helpful to drivers that wants to add its own
> > quirks to sinks.
>
> Why would you want to do that? The point of a shared edid parsing
> code is
> that we can share all these quirks ...
>
> For these kind of patches, always include the driver code that makes
> use
> of your new code too. That makes it much easier to answer these
> questions.
This will be used to disable or enable with quirks PSR in some panels
that do not behave like eDP spec states.
As this would be specifc to i915, I guess is better keep the list only
in i915.
What is your opinion about that?
>
> Thanks, Daniel
>
>
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/drm_edid.c | 20
> > include/drm/drm_edid.h | 1 +
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c
> > b/drivers/gpu/drm/drm_edid.c
> > index b506e3622b08..1a0ddf3d326b 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -1755,6 +1755,21 @@ EXPORT_SYMBOL(drm_edid_duplicate);
> >
> > /*** EDID parsing ***/
> >
> > +/**
> > + * drm_edid_manufacturer_parse - parse the EDID manufacturer id to
> > readable
> > + * characters and set into manufacturer parameter.
> > + * @edid: EDID to get the manufacturer
> > + * @manufacturer: the char buffer to store the id
> > + */
> > +void drm_edid_manufacturer_parse(const struct edid *edid, char
> > manufacturer[3])
> > +{
> > + manufacturer[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > + manufacturer[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > + ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> > + manufacturer[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > +}
> > +EXPORT_SYMBOL(drm_edid_manufacturer_parse);
> > +
> > /**
> > * edid_vendor - match a string against EDID's obfuscated vendor
> > field
> > * @edid: EDID to match
> > @@ -1766,10 +1781,7 @@ static bool edid_vendor(const struct edid
> > *edid, const char *vendor)
> > {
> > char edid_vendor[3];
> >
> > - edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > - edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > - ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> > - edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > + drm_edid_manufacturer_parse(edid, edid_vendor);
> >
> > return !strncmp(edid_vendor, vendor, 3);
> > }
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index e3c404833115..e4f3f7f34d6a 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -466,6 +466,7 @@ struct edid *drm_get_edid_switcheroo(struct
> > drm_connector *connector,
> > struct i2c_adapter *adapter);
> > struct edid *drm_edid_duplicate(const struct edid *edid);
> > int drm_add_edid_modes(struct drm_connector *connector, struct
> > edid *edid);
> > +void drm_edid_manufacturer_parse(const struct edid *edid, char
> > manufacturer[3]);
> >
> > u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
> > enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8
> > video_code);
> > --
> > 2.19.1
> >
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/radeon: remove set but not used variable 'rdev'
On Thu, Nov 8, 2018 at 9:04 AM YueHaibing wrote:
>
> From: Yue Haibing
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/radeon/radeon_object.c: In function 'radeon_bo_unref':
> drivers/gpu/drm/radeon/radeon_object.c:317:24: warning:
> variable 'rdev' set but not used [-Wunused-but-set-variable]
>
> It not used any more after commit
> e7e31600d3e2 ("drm/radeon: remove taking mclk_lock from radeon_bo_unref")
>
> Signed-off-by: Yue Haibing
Applied. thanks!
Alex
> ---
> drivers/gpu/drm/radeon/radeon_object.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c
> b/drivers/gpu/drm/radeon/radeon_object.c
> index ba2fd29..c4b2e14 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -314,11 +314,9 @@ struct radeon_bo *radeon_bo_ref(struct radeon_bo *bo)
> void radeon_bo_unref(struct radeon_bo **bo)
> {
> struct ttm_buffer_object *tbo;
> - struct radeon_device *rdev;
>
> if ((*bo) == NULL)
> return;
> - rdev = (*bo)->rdev;
> tbo = &((*bo)->tbo);
> ttm_bo_put(tbo);
> *bo = NULL;
>
>
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108693] black screen with "drm/amd/display: Do not limit color depth to 8bpc" e03fd3f300f6184c1264186a4c815e93bf658abb >=4.18
https://bugs.freedesktop.org/show_bug.cgi?id=108693 --- Comment #2 from Alex Deucher --- This patchset may help: https://patchwork.freedesktop.org/series/52164/ -- 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 108693] black screen with "drm/amd/display: Do not limit color depth to 8bpc" e03fd3f300f6184c1264186a4c815e93bf658abb >=4.18
https://bugs.freedesktop.org/show_bug.cgi?id=108693 --- Comment #1 from Ronny Standtke --- I can confirm the bug. We provide an exam environment based on Debian Live. Since we switched to Kernel 4.18 (provided in Debian backports) we have seen *many* Macs failing to boot and just showing a black screen. So at least for us this regression is quite severe... -- 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 v2 3/3] drm/vc4: Add a load tracker to prevent HVS underflow errors
Boris Brezillon writes:
> On Thu, 08 Nov 2018 08:26:49 -0800
> Eric Anholt wrote:
>
>> >> > +static void vc4_plane_calc_load(struct drm_plane_state *state)
>> >> > +{
>> >> > + unsigned int hvs_load_shift, vrefresh, i;
>> >> > + struct drm_framebuffer *fb = state->fb;
>> >> > + struct vc4_plane_state *vc4_state;
>> >> > + struct drm_crtc_state *crtc_state;
>> >> > + unsigned int vscale_factor;
>> >> > +
>> >> > + vc4_state = to_vc4_plane_state(state);
>> >> > + crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>> >> > + state->crtc);
>> >> > + vrefresh = drm_mode_vrefresh(&crtc_state->adjusted_mode);
>> >> > +
>> >> > + /* The HVS is able to process 2 pixels/cycle when scaling the
>> >> > source,
>> >> > +* 4 pixels/cycle otherwise.
>> >> > +* Alpha blending step seems to be pipelined and it's always
>> >> > operating
>> >> > +* at 4 pixels/cycle, so the limiting aspect here seems to be
>> >> > the
>> >> > +* scaler block.
>> >> > +* HVS load is expressed in clk-cycles/sec (AKA Hz).
>> >> > +*/
>> >> > + if (vc4_state->x_scaling[0] != VC4_SCALING_NONE ||
>> >> > + vc4_state->x_scaling[1] != VC4_SCALING_NONE ||
>> >> > + vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
>> >> > + vc4_state->y_scaling[1] != VC4_SCALING_NONE)
>> >> > + hvs_load_shift = 1;
>> >> > + else
>> >> > + hvs_load_shift = 2;
>> >> > +
>> >> > + vc4_state->membus_load = 0;
>> >> > + vc4_state->hvs_load = 0;
>> >> > + for (i = 0; i < fb->format->num_planes; i++) {
>> >> > + unsigned long pixels_load;
>> >>
>> >> I'm scared any time I see longs. Do you want 32 or 64 bits here?
>> >
>> > I just assumed a 32bit unsigned var would be enough, so unsigned long
>> > seemed just fine. I can use u32 or u64 if you prefer.
>>
>> Yes, please. See also Maxime's recent trouble with a 64-bit kernel.
>
> Will use u32 then.
>
>>
>> >> > + /* Even if the bandwidth/plane required for a single
>> >> > frame is
>> >> > +*
>> >> > +* vc4_state->src_w[i] * vc4_state->src_h[i] * cpp *
>> >> > vrefresh
>> >> > +*
>> >> > +* when downscaling, we have to read more pixels per
>> >> > line in
>> >> > +* the time frame reserved for a single line, so the
>> >> > bandwidth
>> >> > +* demand can be punctually higher. To account for
>> >> > that, we
>> >> > +* calculate the down-scaling factor and multiply the
>> >> > plane
>> >> > +* load by this number. We're likely over-estimating
>> >> > the read
>> >> > +* demand, but that's better than under-estimating it.
>> >> > +*/
>> >> > + vscale_factor = DIV_ROUND_UP(vc4_state->src_h[i],
>> >> > +vc4_state->crtc_h);
>> >> > + pixels_load = vc4_state->src_w[i] * vc4_state->src_h[i]
>> >> > *
>> >> > + vscale_factor;
>> >>
>> >> If we're upscaling (common for video, right?), aren't we under-counting
>> >> the cost? You need to scale/colorspace-convert crtc_w * crtc_h at 2
>> >> pixels per cycle.
>> >
>> > That's not entirely clear to me. I'm not sure what the scaler does when
>> > upscaling. Are the same pixels read several times from the memory? If
>> > that's the case, then the membus load should indeed be based on the
>> > crtc_w,h.
>>
>> I'm going to punt on this question because that would be a *lot* of
>> verilog tracing to figure out for me (and I'm not sure I'd even trust
>> what I came up with).
>>
>> > Also, when the spec says the HVS can process 4pixels/cycles, is it 4
>> > input pixels or 4 output pixels per cycle?
>>
>> Well, it's 4 pixels per cycle when not scaling, so both :)
>
> Sorry, I meant 2pixels/cycle :).
>
>>
>> I think the scaling pipeline is doing two output pixels per cycle.
>> Nothing else would make sense to me.
>
> Okay, so the HVS load should be based on crtc_w/h and the membus load
> should be based on src_w/h and the scaling ratio, right?
That sounds about right to me.
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()
Hi,
On Tue, Nov 06, 2018 at 07:28:02AM -0800, Tony Lindgren wrote:
> We're missing a call to of_platform_depopulate() on errors for dsi.
> Looks like dss is already doing this.
>
> Signed-off-by: Tony Lindgren
> ---
Reviewed-by: Sebastian Reichel
-- Sebastian
> drivers/gpu/drm/omapdrm/dss/dsi.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -5429,15 +5429,19 @@ static int dsi_probe(struct platform_device *pdev)
> }
>
> r = of_platform_populate(dev->of_node, NULL, NULL, dev);
> - if (r)
> + if (r) {
> DSSERR("Failed to populate DSI child devices: %d\n", r);
> + goto err_uninit_output;
> + }
>
> r = component_add(&pdev->dev, &dsi_component_ops);
> if (r)
> - goto err_uninit_output;
> + goto err_of_depopulate;
>
> return 0;
>
> +err_of_depopulate:
> + of_platform_depopulate(dev);
> err_uninit_output:
> dsi_uninit_output(dsi);
> err_pm_disable:
> --
> 2.19.1
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/omap: dsi: Fix missing of_platform_depopulate()
Hi Tony, On Thursday, 8 November 2018 19:01:45 EET Tony Lindgren wrote: > * Laurent Pinchart [181107 05:30]: > > On Tuesday, 6 November 2018 17:28:02 EET Tony Lindgren wrote: > > > We're missing a call to of_platform_depopulate() on errors for dsi. > > > Looks like dss is already doing this. > > > > > > Signed-off-by: Tony Lindgren > > > > This makes sense to me. You may however want to note in the commit message > > that this patch also turns the of_platform_populate() failures into fatal > > errors. > > Well of_platform_populate() failure should be fatal here,right? :) I'm not complaining about the functional change, just the lack of visibility :-) > Tomi, let me know if you need a resend with updated comments or > if you want to update the message yourself to your liking. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: Revert syncobj timeline changes.
Am 08.11.18 um 17:07 schrieb Koenig, Christian:
Am 08.11.18 um 17:04 schrieb Eric Anholt:
Daniel suggested I submit this, since we're still seeing regressions
from it. This is a revert to before 48197bc564c7 ("drm: add syncobj
timeline support v9") and its followon fixes.
This is a harmless false positive from lockdep, Chouming and I are
already working on a fix.
On the other hand we had enough trouble with that patch, so if it really
bothers you feel free to add my Acked-by: Christian König
and push it.
Christian.
Christian.
Fixes this on first V3D testcase execution:
[ 48.767088]
[ 48.772410] WARNING: possible recursive locking detected
[ 48.39] 4.19.0-rc6+ #489 Not tainted
[ 48.781668]
[ 48.786993] shader_runner/3284 is trying to acquire lock:
[ 48.792408] ce309d7f (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.800714]
[ 48.800714] but task is already holding lock:
[ 48.806559] c5952bd3 (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.814862]
[ 48.814862] other info that might help us debug this:
[ 48.821410] Possible unsafe locking scenario:
[ 48.821410]
[ 48.827338]CPU0
[ 48.829788]
[ 48.832239] lock(&(&array->lock)->rlock);
[ 48.836434] lock(&(&array->lock)->rlock);
[ 48.840640]
[ 48.840640] *** DEADLOCK ***
[ 48.840640]
[ 48.846582] May be due to missing lock nesting notation
[ 130.763560] 1 lock held by cts-runner/3270:
[ 130.767745] #0: 7834b793 (&(&array->lock)->rlock){-...}, at:
dma_fence_add_callback+0x30/0x23c
[ 130.776461]
stack backtrace:
[ 130.780825] CPU: 1 PID: 3270 Comm: cts-runner Not tainted 4.19.0-rc6+ #486
[ 130.787706] Hardware name: Broadcom STB (Flattened Device Tree)
[ 130.793645] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 130.801404] [] (show_stack) from []
(dump_stack+0xa8/0xd4)
[ 130.808642] [] (dump_stack) from []
(__lock_acquire+0x848/0x1a68)
[ 130.816483] [] (__lock_acquire) from []
(lock_acquire+0xd8/0x22c)
[ 130.824326] [] (lock_acquire) from []
(_raw_spin_lock_irqsave+0x54/0x68)
[ 130.832777] [] (_raw_spin_lock_irqsave) from []
(dma_fence_add_callback+0x30/0x23c)
[ 130.842183] [] (dma_fence_add_callback) from []
(dma_fence_array_enable_signaling+0x58/0xec)
[ 130.852371] [] (dma_fence_array_enable_signaling) from
[] (dma_fence_add_callback+0xe8/0x23c)
[ 130.862647] [] (dma_fence_add_callback) from []
(drm_syncobj_wait_ioctl+0x518/0x614)
[ 130.872143] [] (drm_syncobj_wait_ioctl) from []
(drm_ioctl_kernel+0xb0/0xf0)
[ 130.880940] [] (drm_ioctl_kernel) from []
(drm_ioctl+0x1d8/0x390)
[ 130.888782] [] (drm_ioctl) from []
(do_vfs_ioctl+0xb0/0x8ac)
[ 130.896187] [] (do_vfs_ioctl) from []
(ksys_ioctl+0x34/0x60)
[ 130.903593] [] (ksys_ioctl) from []
(ret_fast_syscall+0x0/0x28)
Cc: Chunming Zhou
Cc: Christian König
Cc: Daniel Vetter
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/drm_syncobj.c | 359 +++---
include/drm/drm_syncobj.h | 73 ---
2 files changed, 105 insertions(+), 327 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index da8175d9c6ff..e2c5b3ca4824 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,9 +56,6 @@
#include "drm_internal.h"
#include
-/* merge normal syncobj to timeline syncobj, the point interval is 1 */
-#define DRM_SYNCOBJ_BINARY_POINT 1
-
struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -74,29 +71,7 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops
= {
.get_timeline_name = drm_syncobj_stub_fence_get_name,
};
-struct drm_syncobj_signal_pt {
- struct dma_fence_array *fence_array;
- u64value;
- struct list_head list;
-};
-
-static DEFINE_SPINLOCK(signaled_fence_lock);
-static struct dma_fence signaled_fence;
-static struct dma_fence *drm_syncobj_get_stub_fence(void)
-{
- spin_lock(&signaled_fence_lock);
- if (!signaled_fence.ops) {
- dma_fence_init(&signaled_fence,
- &drm_syncobj_stub_fence_ops,
- &signaled_fence_lock,
- 0, 0);
- dma_fence_signal_locked(&signaled_fence);
- }
- spin_unlock(&signaled_fence_lock);
-
- return dma_fence_get(&signaled_fence);
-}
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -123,27 +98,6 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file
*file_private,
}
EXPORT_SYMBOL(drm_syncobj_find);
-static struct dma_fence *
-drm_syncobj_find_signal_pt_for_point(struct drm_syncobj *syncobj,
-uint64_t point)
-{
- struct drm_syncobj_signal_pt *
Re: [PATCH v2 3/3] drm/vc4: Add a load tracker to prevent HVS underflow errors
On Thu, 08 Nov 2018 08:26:49 -0800
Eric Anholt wrote:
> >> > +static void vc4_plane_calc_load(struct drm_plane_state *state)
> >> > +{
> >> > +unsigned int hvs_load_shift, vrefresh, i;
> >> > +struct drm_framebuffer *fb = state->fb;
> >> > +struct vc4_plane_state *vc4_state;
> >> > +struct drm_crtc_state *crtc_state;
> >> > +unsigned int vscale_factor;
> >> > +
> >> > +vc4_state = to_vc4_plane_state(state);
> >> > +crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> >> > +state->crtc);
> >> > +vrefresh = drm_mode_vrefresh(&crtc_state->adjusted_mode);
> >> > +
> >> > +/* The HVS is able to process 2 pixels/cycle when scaling the
> >> > source,
> >> > + * 4 pixels/cycle otherwise.
> >> > + * Alpha blending step seems to be pipelined and it's always
> >> > operating
> >> > + * at 4 pixels/cycle, so the limiting aspect here seems to be
> >> > the
> >> > + * scaler block.
> >> > + * HVS load is expressed in clk-cycles/sec (AKA Hz).
> >> > + */
> >> > +if (vc4_state->x_scaling[0] != VC4_SCALING_NONE ||
> >> > +vc4_state->x_scaling[1] != VC4_SCALING_NONE ||
> >> > +vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
> >> > +vc4_state->y_scaling[1] != VC4_SCALING_NONE)
> >> > +hvs_load_shift = 1;
> >> > +else
> >> > +hvs_load_shift = 2;
> >> > +
> >> > +vc4_state->membus_load = 0;
> >> > +vc4_state->hvs_load = 0;
> >> > +for (i = 0; i < fb->format->num_planes; i++) {
> >> > +unsigned long pixels_load;
> >>
> >> I'm scared any time I see longs. Do you want 32 or 64 bits here?
> >
> > I just assumed a 32bit unsigned var would be enough, so unsigned long
> > seemed just fine. I can use u32 or u64 if you prefer.
>
> Yes, please. See also Maxime's recent trouble with a 64-bit kernel.
Will use u32 then.
>
> >> > +/* Even if the bandwidth/plane required for a single
> >> > frame is
> >> > + *
> >> > + * vc4_state->src_w[i] * vc4_state->src_h[i] * cpp *
> >> > vrefresh
> >> > + *
> >> > + * when downscaling, we have to read more pixels per
> >> > line in
> >> > + * the time frame reserved for a single line, so the
> >> > bandwidth
> >> > + * demand can be punctually higher. To account for
> >> > that, we
> >> > + * calculate the down-scaling factor and multiply the
> >> > plane
> >> > + * load by this number. We're likely over-estimating
> >> > the read
> >> > + * demand, but that's better than under-estimating it.
> >> > + */
> >> > +vscale_factor = DIV_ROUND_UP(vc4_state->src_h[i],
> >> > + vc4_state->crtc_h);
> >> > +pixels_load = vc4_state->src_w[i] * vc4_state->src_h[i]
> >> > *
> >> > + vscale_factor;
> >>
> >> If we're upscaling (common for video, right?), aren't we under-counting
> >> the cost? You need to scale/colorspace-convert crtc_w * crtc_h at 2
> >> pixels per cycle.
> >
> > That's not entirely clear to me. I'm not sure what the scaler does when
> > upscaling. Are the same pixels read several times from the memory? If
> > that's the case, then the membus load should indeed be based on the
> > crtc_w,h.
>
> I'm going to punt on this question because that would be a *lot* of
> verilog tracing to figure out for me (and I'm not sure I'd even trust
> what I came up with).
>
> > Also, when the spec says the HVS can process 4pixels/cycles, is it 4
> > input pixels or 4 output pixels per cycle?
>
> Well, it's 4 pixels per cycle when not scaling, so both :)
Sorry, I meant 2pixels/cycle :).
>
> I think the scaling pipeline is doing two output pixels per cycle.
> Nothing else would make sense to me.
Okay, so the HVS load should be based on crtc_w/h and the membus load
should be based on src_w/h and the scaling ratio, right?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] Revert "drm/sched: fix timeout handling v2"
Am 08.11.18 um 17:19 schrieb Eric Anholt:
> "Koenig, Christian" writes:
>
>> Am 08.11.18 um 17:04 schrieb Eric Anholt:
>>> This reverts commit 0efd2d2f68cd5dbddf4ecd974c33133257d16a8e. Fixes
>>> this failure in V3D GPU reset:
>>>
>>> [ 1418.227796] Unable to handle kernel NULL pointer dereference at virtual
>>> address 0018
>>> [ 1418.235947] pgd = dc4c55ca
>>> [ 1418.238695] [0018] *pgd=8040004003, *pmd=
>>> [ 1418.244132] Internal error: Oops: 206 [#1] SMP ARM
>>> [ 1418.248934] Modules linked in:
>>> [ 1418.252001] CPU: 0 PID: 10253 Comm: kworker/0:0 Not tainted 4.19.0-rc6+
>>> #486
>>> [ 1418.259058] Hardware name: Broadcom STB (Flattened Device Tree)
>>> [ 1418.265002] Workqueue: events drm_sched_job_timedout
>>> [ 1418.269986] PC is at dma_fence_remove_callback+0x8/0x50
>>> [ 1418.275218] LR is at drm_sched_job_timedout+0x4c/0x118
>>> ...
>>> [ 1418.415891] [] (dma_fence_remove_callback) from []
>>> (drm_sched_job_timedout+0x4c/0x118)
>>> [ 1418.425571] [] (drm_sched_job_timedout) from []
>>> (process_one_work+0x2c8/0x7bc)
>>> [ 1418.434552] [] (process_one_work) from []
>>> (worker_thread+0x44/0x590)
>>> [ 1418.442663] [] (worker_thread) from []
>>> (kthread+0x160/0x168)
>>> [ 1418.450076] [] (kthread) from []
>>> (ret_from_fork+0x14/0x28)
>>>
>>> Cc: Christian König
>>> Cc: Nayan Deshmukh
>>> Cc: Alex Deucher
>>> Signed-off-by: Eric Anholt
>> Well NAK. The problem here is that fence->parent is NULL which is most
>> likely caused by an issue somewhere else.
>>
>> We could easily work around that with an extra NULL check, but reverting
>> the patch would break GPU recovery again.
> My GPU recovery works with the revert and reliably doesn't work without
> it, so my idea of "break GPU recovery" is the opposite of yours. Can
> you help figure out what in this change broke my driver?
The problem is here:
> - list_for_each_entry_reverse(job, &sched->ring_mirror_list, node) {
> - struct drm_sched_fence *fence = job->s_fence;
> -
> - if (!dma_fence_remove_callback(fence->parent, &fence->cb))
> - goto already_signaled;
dma_fence_remove_callback() will fault if fence->parent is NULL. A
simple "if (!fence->parent) continue;" should be enough to work around that.
But I'm not sure how exactly fence->parent became NULL in the first place.
Going to double check the code once more,
Christian.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108049] [vega10] amdgpu fails to either wake up the GPU or while putting it to sleep
https://bugs.freedesktop.org/show_bug.cgi?id=108049 --- Comment #5 from Matthew Miller --- Can confirm: kernel-4.20.0-0.rc1.git1.2.fc30.x86_64 fixes. -- 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 1/2] Revert "drm/sched: fix timeout handling v2"
"Koenig, Christian" writes: > Am 08.11.18 um 17:04 schrieb Eric Anholt: >> This reverts commit 0efd2d2f68cd5dbddf4ecd974c33133257d16a8e. Fixes >> this failure in V3D GPU reset: >> >> [ 1418.227796] Unable to handle kernel NULL pointer dereference at virtual >> address 0018 >> [ 1418.235947] pgd = dc4c55ca >> [ 1418.238695] [0018] *pgd=8040004003, *pmd= >> [ 1418.244132] Internal error: Oops: 206 [#1] SMP ARM >> [ 1418.248934] Modules linked in: >> [ 1418.252001] CPU: 0 PID: 10253 Comm: kworker/0:0 Not tainted 4.19.0-rc6+ >> #486 >> [ 1418.259058] Hardware name: Broadcom STB (Flattened Device Tree) >> [ 1418.265002] Workqueue: events drm_sched_job_timedout >> [ 1418.269986] PC is at dma_fence_remove_callback+0x8/0x50 >> [ 1418.275218] LR is at drm_sched_job_timedout+0x4c/0x118 >> ... >> [ 1418.415891] [] (dma_fence_remove_callback) from [] >> (drm_sched_job_timedout+0x4c/0x118) >> [ 1418.425571] [] (drm_sched_job_timedout) from [] >> (process_one_work+0x2c8/0x7bc) >> [ 1418.434552] [] (process_one_work) from [] >> (worker_thread+0x44/0x590) >> [ 1418.442663] [] (worker_thread) from [] >> (kthread+0x160/0x168) >> [ 1418.450076] [] (kthread) from [] >> (ret_from_fork+0x14/0x28) >> >> Cc: Christian König >> Cc: Nayan Deshmukh >> Cc: Alex Deucher >> Signed-off-by: Eric Anholt > > Well NAK. The problem here is that fence->parent is NULL which is most > likely caused by an issue somewhere else. > > We could easily work around that with an extra NULL check, but reverting > the patch would break GPU recovery again. My GPU recovery works with the revert and reliably doesn't work without it, so my idea of "break GPU recovery" is the opposite of yours. Can you help figure out what in this change broke my driver? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107955] AMDGPU driver keeps reloading on hybrid graphics system causing stuttering.
https://bugs.freedesktop.org/show_bug.cgi?id=107955 --- Comment #28 from Alex Deucher --- (In reply to Ransu from comment #26) > > Does my card support AMDGPU-PRO drivers? If so is there any real advantage > of using the "PRO" extras over the standard open source driver? You only need the "PRO" driver if you need OpenGL that is certified for workstation applications or OpenCL. -- 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 v2 3/3] drm/vc4: Add a load tracker to prevent HVS underflow errors
Boris Brezillon writes:
> Hi Eric,
>
> On Tue, 30 Oct 2018 16:12:55 -0700
> Eric Anholt wrote:
>> > +static void vc4_load_tracker_destroy_state(struct drm_private_obj *obj,
>> > + struct drm_private_state *state)
>> > +{
>> > + struct vc4_load_tracker_state *load_state;
>> > +
>> > + load_state = to_vc4_load_tracker_state(state);
>> > + kfree(load_state);
>> > +}
>>
>> Optional: just kfree(state) for simplicity.
>
> Hm, not sure that's a good idea. kfree(state) works as long as
> drm_private_state is the first field in vc4_load_tracker_state, but it
> sounds a bit fragile.
>
> I can do
>
> kfree(to_vc4_load_tracker_state(state));
>
> if you prefer.
I said optional for that reason :) Just keep it as is.
>> > +static void vc4_plane_calc_load(struct drm_plane_state *state)
>> > +{
>> > + unsigned int hvs_load_shift, vrefresh, i;
>> > + struct drm_framebuffer *fb = state->fb;
>> > + struct vc4_plane_state *vc4_state;
>> > + struct drm_crtc_state *crtc_state;
>> > + unsigned int vscale_factor;
>> > +
>> > + vc4_state = to_vc4_plane_state(state);
>> > + crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>> > + state->crtc);
>> > + vrefresh = drm_mode_vrefresh(&crtc_state->adjusted_mode);
>> > +
>> > + /* The HVS is able to process 2 pixels/cycle when scaling the source,
>> > + * 4 pixels/cycle otherwise.
>> > + * Alpha blending step seems to be pipelined and it's always operating
>> > + * at 4 pixels/cycle, so the limiting aspect here seems to be the
>> > + * scaler block.
>> > + * HVS load is expressed in clk-cycles/sec (AKA Hz).
>> > + */
>> > + if (vc4_state->x_scaling[0] != VC4_SCALING_NONE ||
>> > + vc4_state->x_scaling[1] != VC4_SCALING_NONE ||
>> > + vc4_state->y_scaling[0] != VC4_SCALING_NONE ||
>> > + vc4_state->y_scaling[1] != VC4_SCALING_NONE)
>> > + hvs_load_shift = 1;
>> > + else
>> > + hvs_load_shift = 2;
>> > +
>> > + vc4_state->membus_load = 0;
>> > + vc4_state->hvs_load = 0;
>> > + for (i = 0; i < fb->format->num_planes; i++) {
>> > + unsigned long pixels_load;
>>
>> I'm scared any time I see longs. Do you want 32 or 64 bits here?
>
> I just assumed a 32bit unsigned var would be enough, so unsigned long
> seemed just fine. I can use u32 or u64 if you prefer.
Yes, please. See also Maxime's recent trouble with a 64-bit kernel.
>> > + /* Even if the bandwidth/plane required for a single frame is
>> > + *
>> > + * vc4_state->src_w[i] * vc4_state->src_h[i] * cpp * vrefresh
>> > + *
>> > + * when downscaling, we have to read more pixels per line in
>> > + * the time frame reserved for a single line, so the bandwidth
>> > + * demand can be punctually higher. To account for that, we
>> > + * calculate the down-scaling factor and multiply the plane
>> > + * load by this number. We're likely over-estimating the read
>> > + * demand, but that's better than under-estimating it.
>> > + */
>> > + vscale_factor = DIV_ROUND_UP(vc4_state->src_h[i],
>> > + vc4_state->crtc_h);
>> > + pixels_load = vc4_state->src_w[i] * vc4_state->src_h[i] *
>> > +vscale_factor;
>>
>> If we're upscaling (common for video, right?), aren't we under-counting
>> the cost? You need to scale/colorspace-convert crtc_w * crtc_h at 2
>> pixels per cycle.
>
> That's not entirely clear to me. I'm not sure what the scaler does when
> upscaling. Are the same pixels read several times from the memory? If
> that's the case, then the membus load should indeed be based on the
> crtc_w,h.
I'm going to punt on this question because that would be a *lot* of
verilog tracing to figure out for me (and I'm not sure I'd even trust
what I came up with).
> Also, when the spec says the HVS can process 4pixels/cycles, is it 4
> input pixels or 4 output pixels per cycle?
Well, it's 4 pixels per cycle when not scaling, so both :)
I think the scaling pipeline is doing two output pixels per cycle.
Nothing else would make sense to me.
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/4] V3D TFU engine support
This series adds support for V3D's TFU engine (a little texture tiling/YUV import/mipmap generation block). Corresponding Mesa userspace is at https://gitlab.freedesktop.org/anholt/mesa/commits/v3d-tfu Once we have TFU, the next step will be compute shaders, which are a lot more interesting. Eric Anholt (4): drm/v3d: Fix whitespace inconsistency in the header. drm/v3d: Update a comment about what uses v3d_job_dependency(). drm/v3d: Clean up the reservation object setup. drm/v3d: Add support for submitting jobs to the TFU. drivers/gpu/drm/v3d/v3d_drv.c | 12 +- drivers/gpu/drm/v3d/v3d_drv.h | 32 +- drivers/gpu/drm/v3d/v3d_gem.c | 193 ++-- drivers/gpu/drm/v3d/v3d_irq.c | 12 +- drivers/gpu/drm/v3d/v3d_regs.h | 58 ++ drivers/gpu/drm/v3d/v3d_sched.c | 149 drivers/gpu/drm/v3d/v3d_trace.h | 20 include/uapi/drm/v3d_drm.h | 29 - 8 files changed, 437 insertions(+), 68 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/v3d: Clean up the reservation object setup.
The extra to_v3d_bo() calls came from copying this from the vc4
driver, which stored the cma gem object in the structs.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_gem.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index b88c96911453..d0dfdcbbd42c 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -214,10 +214,8 @@ v3d_attach_object_fences(struct v3d_exec_info *exec)
int i;
for (i = 0; i < exec->bo_count; i++) {
- bo = to_v3d_bo(&exec->bo[i]->base);
-
/* XXX: Use shared fences for read-only objects. */
- reservation_object_add_excl_fence(bo->resv, out_fence);
+ reservation_object_add_excl_fence(exec->bo[i]->resv, out_fence);
}
}
@@ -228,11 +226,8 @@ v3d_unlock_bo_reservations(struct drm_device *dev,
{
int i;
- for (i = 0; i < exec->bo_count; i++) {
- struct v3d_bo *bo = to_v3d_bo(&exec->bo[i]->base);
-
- ww_mutex_unlock(&bo->resv->lock);
- }
+ for (i = 0; i < exec->bo_count; i++)
+ ww_mutex_unlock(&exec->bo[i]->resv->lock);
ww_acquire_fini(acquire_ctx);
}
@@ -251,13 +246,13 @@ v3d_lock_bo_reservations(struct drm_device *dev,
{
int contended_lock = -1;
int i, ret;
- struct v3d_bo *bo;
ww_acquire_init(acquire_ctx, &reservation_ww_class);
retry:
if (contended_lock != -1) {
- bo = to_v3d_bo(&exec->bo[contended_lock]->base);
+ struct v3d_bo *bo = exec->bo[contended_lock];
+
ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock,
acquire_ctx);
if (ret) {
@@ -270,19 +265,16 @@ v3d_lock_bo_reservations(struct drm_device *dev,
if (i == contended_lock)
continue;
- bo = to_v3d_bo(&exec->bo[i]->base);
-
- ret = ww_mutex_lock_interruptible(&bo->resv->lock, acquire_ctx);
+ ret = ww_mutex_lock_interruptible(&exec->bo[i]->resv->lock,
+ acquire_ctx);
if (ret) {
int j;
- for (j = 0; j < i; j++) {
- bo = to_v3d_bo(&exec->bo[j]->base);
- ww_mutex_unlock(&bo->resv->lock);
- }
+ for (j = 0; j < i; j++)
+ ww_mutex_unlock(&exec->bo[j]->resv->lock);
if (contended_lock != -1 && contended_lock >= i) {
- bo = to_v3d_bo(&exec->bo[contended_lock]->base);
+ struct v3d_bo *bo = exec->bo[contended_lock];
ww_mutex_unlock(&bo->resv->lock);
}
@@ -303,9 +295,7 @@ v3d_lock_bo_reservations(struct drm_device *dev,
* before we commit the CL to the hardware.
*/
for (i = 0; i < exec->bo_count; i++) {
- bo = to_v3d_bo(&exec->bo[i]->base);
-
- ret = reservation_object_reserve_shared(bo->resv, 1);
+ ret = reservation_object_reserve_shared(exec->bo[i]->resv, 1);
if (ret) {
v3d_unlock_bo_reservations(dev, exec, acquire_ctx);
return ret;
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/v3d: Add support for submitting jobs to the TFU.
The TFU can copy from raster, UIF, and SAND input images to UIF output
images, with optional mipmap generation. This will certainly be
useful for media EGL image input, but is also useful immediately for
mipmap generation without bogging the V3D core down.
For now we only run the queue 1 job deep, and don't have any hang
recovery (though I don't think we should need it, with TFU). Queuing
multiple jobs in the HW will require synchronizing the YUV coefficient
regs updates since they don't get FIFOed with the job.
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/v3d/v3d_drv.c | 12 ++-
drivers/gpu/drm/v3d/v3d_drv.h | 32 +-
drivers/gpu/drm/v3d/v3d_gem.c | 177
drivers/gpu/drm/v3d/v3d_irq.c | 12 ++-
drivers/gpu/drm/v3d/v3d_regs.h | 58 +++
drivers/gpu/drm/v3d/v3d_sched.c | 147 ++
drivers/gpu/drm/v3d/v3d_trace.h | 20
include/uapi/drm/v3d_drm.h | 25 +
8 files changed, 431 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 4857c0a63131..da0863281a73 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -184,10 +184,15 @@ static int v3d_get_param_ioctl(struct drm_device *dev,
void *data,
return 0;
}
- /* Any params that aren't just register reads would go here. */
- DRM_DEBUG("Unknown parameter %d\n", args->param);
- return -EINVAL;
+ switch (args->param) {
+ case DRM_V3D_PARAM_SUPPORTS_TFU:
+ args->value = 1;
+ return 0;
+ default:
+ DRM_DEBUG("Unknown parameter %d\n", args->param);
+ return -EINVAL;
+ }
}
static int
@@ -251,6 +256,7 @@ static const struct drm_ioctl_desc v3d_drm_ioctls[] = {
DRM_IOCTL_DEF_DRV(V3D_MMAP_BO, v3d_mmap_bo_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(V3D_GET_PARAM, v3d_get_param_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(V3D_GET_BO_OFFSET, v3d_get_bo_offset_ioctl,
DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(V3D_SUBMIT_TFU, v3d_submit_tfu_ioctl,
DRM_RENDER_ALLOW | DRM_AUTH),
};
static const struct vm_operations_struct v3d_vm_ops = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 83c55ab6e1c0..e0624ea72942 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -8,19 +8,18 @@
#include
#include
#include
+#include "uapi/drm/v3d_drm.h"
#define GMP_GRANULARITY (128 * 1024)
-/* Enum for each of the V3D queues. We maintain various queue
- * tracking as an array because at some point we'll want to support
- * the TFU (texture formatting unit) as another queue.
- */
+/* Enum for each of the V3D queues. */
enum v3d_queue {
V3D_BIN,
V3D_RENDER,
+ V3D_TFU,
};
-#define V3D_MAX_QUEUES (V3D_RENDER + 1)
+#define V3D_MAX_QUEUES (V3D_TFU + 1)
struct v3d_queue_state {
struct drm_gpu_scheduler sched;
@@ -74,6 +73,7 @@ struct v3d_dev {
struct v3d_exec_info *bin_job;
struct v3d_exec_info *render_job;
+ struct v3d_tfu_job *tfu_job;
struct v3d_queue_state queue[V3D_MAX_QUEUES];
@@ -224,6 +224,25 @@ struct v3d_exec_info {
u32 qma, qms, qts;
};
+struct v3d_tfu_job {
+ struct drm_sched_job base;
+
+ struct drm_v3d_submit_tfu args;
+
+ /* An optional fence userspace can pass in for the job to depend on. */
+ struct dma_fence *in_fence;
+
+ /* v3d fence to be signaled by IRQ handler when the job is complete. */
+ struct dma_fence *done_fence;
+
+ struct v3d_dev *v3d;
+
+ struct kref refcount;
+
+ /* This is the array of BOs that were looked up at the start of exec. */
+ struct v3d_bo *bo[4];
+};
+
/**
* _wait_for - magic (register) wait macro
*
@@ -287,9 +306,12 @@ int v3d_gem_init(struct drm_device *dev);
void v3d_gem_destroy(struct drm_device *dev);
int v3d_submit_cl_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+int v3d_submit_tfu_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_priv);
int v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
void v3d_exec_put(struct v3d_exec_info *exec);
+void v3d_tfu_job_put(struct v3d_tfu_job *exec);
void v3d_reset(struct v3d_dev *v3d);
void v3d_invalidate_caches(struct v3d_dev *v3d);
void v3d_flush_caches(struct v3d_dev *v3d);
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index d0dfdcbbd42c..adc8b1ec15e3 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -207,27 +207,27 @@ v3d_flush_caches(struct v3d_dev *v3d)
}
static void
-v3d_attach_object_fences(struct v3d_exec_info *exec)
+v3d_attach_object_fences(struct v3d_bo **bos, int bo_count,
+struct dma_fence *fence)
{
- st
[PATCH 1/4] drm/v3d: Fix whitespace inconsistency in the header.
Signed-off-by: Eric Anholt
---
include/uapi/drm/v3d_drm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index f446656d00b1..b1e5de076b0f 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -66,7 +66,7 @@ struct drm_v3d_submit_cl {
*/
__u32 bcl_start;
-/** End address of the BCL (first byte after the BCL) */
+ /** End address of the BCL (first byte after the BCL) */
__u32 bcl_end;
/* Offset of the render command list.
@@ -82,7 +82,7 @@ struct drm_v3d_submit_cl {
*/
__u32 rcl_start;
-/** End address of the RCL (first byte after the RCL) */
+ /** End address of the RCL (first byte after the RCL) */
__u32 rcl_end;
/** An optional sync object to wait on before starting the BCL. */
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/v3d: Update a comment about what uses v3d_job_dependency().
I merged bin and render's paths in a late refactoring. Signed-off-by: Eric Anholt --- drivers/gpu/drm/v3d/v3d_sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c index 9243dea6e6ad..e1f2aab0717b 100644 --- a/drivers/gpu/drm/v3d/v3d_sched.c +++ b/drivers/gpu/drm/v3d/v3d_sched.c @@ -39,7 +39,7 @@ v3d_job_free(struct drm_sched_job *sched_job) } /** - * Returns the fences that the bin job depends on, one by one. + * Returns the fences that the bin or render job depends on, one by one. * v3d_job_run() won't be called until all of them have been signaled. */ static struct dma_fence * -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] Revert "drm/sched: fix timeout handling v2"
Am 08.11.18 um 17:04 schrieb Eric Anholt:
> This reverts commit 0efd2d2f68cd5dbddf4ecd974c33133257d16a8e. Fixes
> this failure in V3D GPU reset:
>
> [ 1418.227796] Unable to handle kernel NULL pointer dereference at virtual
> address 0018
> [ 1418.235947] pgd = dc4c55ca
> [ 1418.238695] [0018] *pgd=8040004003, *pmd=
> [ 1418.244132] Internal error: Oops: 206 [#1] SMP ARM
> [ 1418.248934] Modules linked in:
> [ 1418.252001] CPU: 0 PID: 10253 Comm: kworker/0:0 Not tainted 4.19.0-rc6+
> #486
> [ 1418.259058] Hardware name: Broadcom STB (Flattened Device Tree)
> [ 1418.265002] Workqueue: events drm_sched_job_timedout
> [ 1418.269986] PC is at dma_fence_remove_callback+0x8/0x50
> [ 1418.275218] LR is at drm_sched_job_timedout+0x4c/0x118
> ...
> [ 1418.415891] [] (dma_fence_remove_callback) from []
> (drm_sched_job_timedout+0x4c/0x118)
> [ 1418.425571] [] (drm_sched_job_timedout) from []
> (process_one_work+0x2c8/0x7bc)
> [ 1418.434552] [] (process_one_work) from []
> (worker_thread+0x44/0x590)
> [ 1418.442663] [] (worker_thread) from []
> (kthread+0x160/0x168)
> [ 1418.450076] [] (kthread) from []
> (ret_from_fork+0x14/0x28)
>
> Cc: Christian König
> Cc: Nayan Deshmukh
> Cc: Alex Deucher
> Signed-off-by: Eric Anholt
Well NAK. The problem here is that fence->parent is NULL which is most
likely caused by an issue somewhere else.
We could easily work around that with an extra NULL check, but reverting
the patch would break GPU recovery again.
Christian.
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 30 +-
> 1 file changed, 1 insertion(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index 44fe587aaef9..bd7d11c47202 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -249,41 +249,13 @@ static void drm_sched_job_timedout(struct work_struct
> *work)
> {
> struct drm_gpu_scheduler *sched;
> struct drm_sched_job *job;
> - int r;
>
> sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
> -
> - spin_lock(&sched->job_list_lock);
> - list_for_each_entry_reverse(job, &sched->ring_mirror_list, node) {
> - struct drm_sched_fence *fence = job->s_fence;
> -
> - if (!dma_fence_remove_callback(fence->parent, &fence->cb))
> - goto already_signaled;
> - }
> -
> job = list_first_entry_or_null(&sched->ring_mirror_list,
> struct drm_sched_job, node);
> - spin_unlock(&sched->job_list_lock);
>
> if (job)
> - sched->ops->timedout_job(job);
> -
> - spin_lock(&sched->job_list_lock);
> - list_for_each_entry(job, &sched->ring_mirror_list, node) {
> - struct drm_sched_fence *fence = job->s_fence;
> -
> - if (!fence->parent || !list_empty(&fence->cb.node))
> - continue;
> -
> - r = dma_fence_add_callback(fence->parent, &fence->cb,
> -drm_sched_process_job);
> - if (r)
> - drm_sched_process_job(fence->parent, &fence->cb);
> -
> -already_signaled:
> - ;
> - }
> - spin_unlock(&sched->job_list_lock);
> + job->sched->ops->timedout_job(job);
> }
>
> /**
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm: Revert syncobj timeline changes.
Am 08.11.18 um 17:04 schrieb Eric Anholt:
> Daniel suggested I submit this, since we're still seeing regressions
> from it. This is a revert to before 48197bc564c7 ("drm: add syncobj
> timeline support v9") and its followon fixes.
This is a harmless false positive from lockdep, Chouming and I are
already working on a fix.
Christian.
>
> Fixes this on first V3D testcase execution:
>
> [ 48.767088]
> [ 48.772410] WARNING: possible recursive locking detected
> [ 48.39] 4.19.0-rc6+ #489 Not tainted
> [ 48.781668]
> [ 48.786993] shader_runner/3284 is trying to acquire lock:
> [ 48.792408] ce309d7f (&(&array->lock)->rlock){}, at:
> dma_fence_add_callback+0x30/0x23c
> [ 48.800714]
> [ 48.800714] but task is already holding lock:
> [ 48.806559] c5952bd3 (&(&array->lock)->rlock){}, at:
> dma_fence_add_callback+0x30/0x23c
> [ 48.814862]
> [ 48.814862] other info that might help us debug this:
> [ 48.821410] Possible unsafe locking scenario:
> [ 48.821410]
> [ 48.827338]CPU0
> [ 48.829788]
> [ 48.832239] lock(&(&array->lock)->rlock);
> [ 48.836434] lock(&(&array->lock)->rlock);
> [ 48.840640]
> [ 48.840640] *** DEADLOCK ***
> [ 48.840640]
> [ 48.846582] May be due to missing lock nesting notation
> [ 130.763560] 1 lock held by cts-runner/3270:
> [ 130.767745] #0: 7834b793 (&(&array->lock)->rlock){-...}, at:
> dma_fence_add_callback+0x30/0x23c
> [ 130.776461]
> stack backtrace:
> [ 130.780825] CPU: 1 PID: 3270 Comm: cts-runner Not tainted 4.19.0-rc6+ #486
> [ 130.787706] Hardware name: Broadcom STB (Flattened Device Tree)
> [ 130.793645] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 130.801404] [] (show_stack) from []
> (dump_stack+0xa8/0xd4)
> [ 130.808642] [] (dump_stack) from []
> (__lock_acquire+0x848/0x1a68)
> [ 130.816483] [] (__lock_acquire) from []
> (lock_acquire+0xd8/0x22c)
> [ 130.824326] [] (lock_acquire) from []
> (_raw_spin_lock_irqsave+0x54/0x68)
> [ 130.832777] [] (_raw_spin_lock_irqsave) from []
> (dma_fence_add_callback+0x30/0x23c)
> [ 130.842183] [] (dma_fence_add_callback) from []
> (dma_fence_array_enable_signaling+0x58/0xec)
> [ 130.852371] [] (dma_fence_array_enable_signaling) from
> [] (dma_fence_add_callback+0xe8/0x23c)
> [ 130.862647] [] (dma_fence_add_callback) from []
> (drm_syncobj_wait_ioctl+0x518/0x614)
> [ 130.872143] [] (drm_syncobj_wait_ioctl) from []
> (drm_ioctl_kernel+0xb0/0xf0)
> [ 130.880940] [] (drm_ioctl_kernel) from []
> (drm_ioctl+0x1d8/0x390)
> [ 130.888782] [] (drm_ioctl) from []
> (do_vfs_ioctl+0xb0/0x8ac)
> [ 130.896187] [] (do_vfs_ioctl) from []
> (ksys_ioctl+0x34/0x60)
> [ 130.903593] [] (ksys_ioctl) from []
> (ret_fast_syscall+0x0/0x28)
>
> Cc: Chunming Zhou
> Cc: Christian König
> Cc: Daniel Vetter
> Signed-off-by: Eric Anholt
> ---
> drivers/gpu/drm/drm_syncobj.c | 359 +++---
> include/drm/drm_syncobj.h | 73 ---
> 2 files changed, 105 insertions(+), 327 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index da8175d9c6ff..e2c5b3ca4824 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -56,9 +56,6 @@
> #include "drm_internal.h"
> #include
>
> -/* merge normal syncobj to timeline syncobj, the point interval is 1 */
> -#define DRM_SYNCOBJ_BINARY_POINT 1
> -
> struct drm_syncobj_stub_fence {
> struct dma_fence base;
> spinlock_t lock;
> @@ -74,29 +71,7 @@ static const struct dma_fence_ops
> drm_syncobj_stub_fence_ops = {
> .get_timeline_name = drm_syncobj_stub_fence_get_name,
> };
>
> -struct drm_syncobj_signal_pt {
> - struct dma_fence_array *fence_array;
> - u64value;
> - struct list_head list;
> -};
> -
> -static DEFINE_SPINLOCK(signaled_fence_lock);
> -static struct dma_fence signaled_fence;
>
> -static struct dma_fence *drm_syncobj_get_stub_fence(void)
> -{
> - spin_lock(&signaled_fence_lock);
> - if (!signaled_fence.ops) {
> - dma_fence_init(&signaled_fence,
> -&drm_syncobj_stub_fence_ops,
> -&signaled_fence_lock,
> -0, 0);
> - dma_fence_signal_locked(&signaled_fence);
> - }
> - spin_unlock(&signaled_fence_lock);
> -
> - return dma_fence_get(&signaled_fence);
> -}
> /**
>* drm_syncobj_find - lookup and reference a sync object.
>* @file_private: drm file private pointer
> @@ -123,27 +98,6 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file
> *file_private,
> }
> EXPORT_SYMBOL(drm_syncobj_find);
>
> -static struct dma_fence *
> -drm_syncobj_find_signal_pt_for_point(struct drm_syncobj *syncobj,
> - uint64_t point)
> -{
> - struct drm_syncobj_signal_pt *signal_pt;
> -
>
[Bug 107955] AMDGPU driver keeps reloading on hybrid graphics system causing stuttering.
https://bugs.freedesktop.org/show_bug.cgi?id=107955 --- Comment #27 from Ransu --- Oh I also did add the kernel modules as follows to my mkinitcpio configuration in case that helped any, first three are for the two GPU my laptop has and the rest are for the encrypted disk. MODULES="i915 amdgpu radeon dm_mod dm_crypt ext4 aes_x86_64 sha256 sha512" -- 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
[PULL] drm-misc-next
Hey Dave, Try #2! Same as try #1, but with less syncobj timeline, and more explicit use of old/new state in atomic core. drm-misc-next-2018-11-08: drm-misc-next for v4.21, part 1 try 2: UAPI Changes: - Revert syncobj timeline support to drm. Cross-subsystem Changes: - Remove shared fence staging in dma-buf's fence object, and allow reserving more than 1 fence and add more paranoia when debugging. - Constify infoframe functions in video/hdmi. Core Changes: - Add vkms todo, and a lot of assorted doc fixes. - Drop transitional helpers and convert drivers to use drm_atomic_helper_shutdown(). - Move atomic state helper functions to drm_atomic_state_helper.[ch] - Refactor drm selftests, and add new tests. - DP MST atomic state cleanups. - Drop EXPORT_SYMBOL from drm leases. - Lease cleanups and fixes. - Create render node for vgem. - Use explicit state in atomic core functions. Driver Changes: - Fix build failure in imx without fbdev emulation. - Add rotation quirk for GPD win2 panel. - Add support for various CDTech panels, Banana Pi Panel, DLC1010GIG, Olimex LCD-O-LinuXino, Samsung S6D16D0, Truly NT35597 WQXGA, Himax HX8357D, simulated RTSM AEMv8. - Add dw_hdmi support to rockchip driver. - Fix YUV support in vc4. - Fix resource id handling in virtio. - Make rockchip use dw-mipi-dsi bridge driver, and add dual dsi support. - Advertise that tinydrm only supports DRM_FORMAT_MOD_LINEAR. - Convert many drivers to use atomic helpers, and drm_fbdev_generic_setup(). - Add Mali linear tiled formats, and enable them in the Mali-DP driver. - Add support for H6 DE3 mixer 0, DW HDMI, HDMI PHY and TCON TOP. - Assorted driver cleanups and fixes. The following changes since commit f2bfc71aee75feff33ca659322b72ffeed5a243d: Merge tag 'drm-intel-next-fixes-2018-10-18' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2018-10-19 14:28:11 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2018-11-08 for you to fetch changes up to 783195ec1cada862d54dee8f312a60bcbba5c0e4: drm/syncobj: disable the timeline UAPI for now v2 (2018-11-08 11:31:34 +0100) drm-misc-next for v4.21, part 1 try 2: UAPI Changes: - Revert syncobj timeline support to drm. Cross-subsystem Changes: - Remove shared fence staging in dma-buf's fence object, and allow reserving more than 1 fence and add more paranoia when debugging. - Constify infoframe functions in video/hdmi. Core Changes: - Add vkms todo, and a lot of assorted doc fixes. - Drop transitional helpers and convert drivers to use drm_atomic_helper_shutdown(). - Move atomic state helper functions to drm_atomic_state_helper.[ch] - Refactor drm selftests, and add new tests. - DP MST atomic state cleanups. - Drop EXPORT_SYMBOL from drm leases. - Lease cleanups and fixes. - Create render node for vgem. - Use explicit state in atomic core functions. Driver Changes: - Fix build failure in imx without fbdev emulation. - Add rotation quirk for GPD win2 panel. - Add support for various CDTech panels, Banana Pi Panel, DLC1010GIG, Olimex LCD-O-LinuXino, Samsung S6D16D0, Truly NT35597 WQXGA, Himax HX8357D, simulated RTSM AEMv8. - Add dw_hdmi support to rockchip driver. - Fix YUV support in vc4. - Fix resource id handling in virtio. - Make rockchip use dw-mipi-dsi bridge driver, and add dual dsi support. - Advertise that tinydrm only supports DRM_FORMAT_MOD_LINEAR. - Convert many drivers to use atomic helpers, and drm_fbdev_generic_setup(). - Add Mali linear tiled formats, and enable them in the Mali-DP driver. - Add support for H6 DE3 mixer 0, DW HDMI, HDMI PHY and TCON TOP. - Assorted driver cleanups and fixes. Aaron Ma (2): vgaarb: Add support for 64-bit frame buffer address vgaarb: Keep adding VGA device in queue Abhinav Kumar (2): drm/panel: Add support for Truly NT35597 panel driver dt-bindings: Add Truly NT35597 panel driver bindings Alexandru Gheorghe (9): drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation drm/selftest: Refactor test-drm_plane_helper drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info drm/fourcc: Add fourcc for Mali linear tiled formats drm: mali-dp: Enable Mali-DP tiled buffer formats drm: Extend framebuffer_check to handle formats with cpp/char_per_block 0 drm/selftests: Add tests for drm_format_info* helpers drm: Add macro to export functions only when CONFIG_DRM_DEBUG_SELFTEST is enabled drm/selftests: Add tests for drm_internal_framebuffer_create Alexandru-Cosmin Gheorghe (1): drm/selftests: Fix build warning -Wframe-larger-than Andrzej Hajda (1): drm/panel: simple: fix BOE/HV070WSA-100 timings Arnd Bergmann (1): drm/imx: fix build failure without CONFIG_DRM_FBDEV_EMULATION Benjamin Gaignard (2):
[PATCH 2/2] drm: Revert syncobj timeline changes.
Daniel suggested I submit this, since we're still seeing regressions
from it. This is a revert to before 48197bc564c7 ("drm: add syncobj
timeline support v9") and its followon fixes.
Fixes this on first V3D testcase execution:
[ 48.767088]
[ 48.772410] WARNING: possible recursive locking detected
[ 48.39] 4.19.0-rc6+ #489 Not tainted
[ 48.781668]
[ 48.786993] shader_runner/3284 is trying to acquire lock:
[ 48.792408] ce309d7f (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.800714]
[ 48.800714] but task is already holding lock:
[ 48.806559] c5952bd3 (&(&array->lock)->rlock){}, at:
dma_fence_add_callback+0x30/0x23c
[ 48.814862]
[ 48.814862] other info that might help us debug this:
[ 48.821410] Possible unsafe locking scenario:
[ 48.821410]
[ 48.827338]CPU0
[ 48.829788]
[ 48.832239] lock(&(&array->lock)->rlock);
[ 48.836434] lock(&(&array->lock)->rlock);
[ 48.840640]
[ 48.840640] *** DEADLOCK ***
[ 48.840640]
[ 48.846582] May be due to missing lock nesting notation
[ 130.763560] 1 lock held by cts-runner/3270:
[ 130.767745] #0: 7834b793 (&(&array->lock)->rlock){-...}, at:
dma_fence_add_callback+0x30/0x23c
[ 130.776461]
stack backtrace:
[ 130.780825] CPU: 1 PID: 3270 Comm: cts-runner Not tainted 4.19.0-rc6+ #486
[ 130.787706] Hardware name: Broadcom STB (Flattened Device Tree)
[ 130.793645] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 130.801404] [] (show_stack) from []
(dump_stack+0xa8/0xd4)
[ 130.808642] [] (dump_stack) from []
(__lock_acquire+0x848/0x1a68)
[ 130.816483] [] (__lock_acquire) from []
(lock_acquire+0xd8/0x22c)
[ 130.824326] [] (lock_acquire) from []
(_raw_spin_lock_irqsave+0x54/0x68)
[ 130.832777] [] (_raw_spin_lock_irqsave) from []
(dma_fence_add_callback+0x30/0x23c)
[ 130.842183] [] (dma_fence_add_callback) from []
(dma_fence_array_enable_signaling+0x58/0xec)
[ 130.852371] [] (dma_fence_array_enable_signaling) from
[] (dma_fence_add_callback+0xe8/0x23c)
[ 130.862647] [] (dma_fence_add_callback) from []
(drm_syncobj_wait_ioctl+0x518/0x614)
[ 130.872143] [] (drm_syncobj_wait_ioctl) from []
(drm_ioctl_kernel+0xb0/0xf0)
[ 130.880940] [] (drm_ioctl_kernel) from []
(drm_ioctl+0x1d8/0x390)
[ 130.888782] [] (drm_ioctl) from []
(do_vfs_ioctl+0xb0/0x8ac)
[ 130.896187] [] (do_vfs_ioctl) from []
(ksys_ioctl+0x34/0x60)
[ 130.903593] [] (ksys_ioctl) from []
(ret_fast_syscall+0x0/0x28)
Cc: Chunming Zhou
Cc: Christian König
Cc: Daniel Vetter
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/drm_syncobj.c | 359 +++---
include/drm/drm_syncobj.h | 73 ---
2 files changed, 105 insertions(+), 327 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index da8175d9c6ff..e2c5b3ca4824 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,9 +56,6 @@
#include "drm_internal.h"
#include
-/* merge normal syncobj to timeline syncobj, the point interval is 1 */
-#define DRM_SYNCOBJ_BINARY_POINT 1
-
struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -74,29 +71,7 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops
= {
.get_timeline_name = drm_syncobj_stub_fence_get_name,
};
-struct drm_syncobj_signal_pt {
- struct dma_fence_array *fence_array;
- u64value;
- struct list_head list;
-};
-
-static DEFINE_SPINLOCK(signaled_fence_lock);
-static struct dma_fence signaled_fence;
-static struct dma_fence *drm_syncobj_get_stub_fence(void)
-{
- spin_lock(&signaled_fence_lock);
- if (!signaled_fence.ops) {
- dma_fence_init(&signaled_fence,
- &drm_syncobj_stub_fence_ops,
- &signaled_fence_lock,
- 0, 0);
- dma_fence_signal_locked(&signaled_fence);
- }
- spin_unlock(&signaled_fence_lock);
-
- return dma_fence_get(&signaled_fence);
-}
/**
* drm_syncobj_find - lookup and reference a sync object.
* @file_private: drm file private pointer
@@ -123,27 +98,6 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file
*file_private,
}
EXPORT_SYMBOL(drm_syncobj_find);
-static struct dma_fence *
-drm_syncobj_find_signal_pt_for_point(struct drm_syncobj *syncobj,
-uint64_t point)
-{
- struct drm_syncobj_signal_pt *signal_pt;
-
- if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) &&
- (point <= syncobj->timeline))
- return drm_syncobj_get_stub_fence();
-
- list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) {
- if (point > signal_pt->value)
- continue;
- if ((syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) &&
-
[PATCH 1/2] Revert "drm/sched: fix timeout handling v2"
This reverts commit 0efd2d2f68cd5dbddf4ecd974c33133257d16a8e. Fixes
this failure in V3D GPU reset:
[ 1418.227796] Unable to handle kernel NULL pointer dereference at virtual
address 0018
[ 1418.235947] pgd = dc4c55ca
[ 1418.238695] [0018] *pgd=8040004003, *pmd=
[ 1418.244132] Internal error: Oops: 206 [#1] SMP ARM
[ 1418.248934] Modules linked in:
[ 1418.252001] CPU: 0 PID: 10253 Comm: kworker/0:0 Not tainted 4.19.0-rc6+ #486
[ 1418.259058] Hardware name: Broadcom STB (Flattened Device Tree)
[ 1418.265002] Workqueue: events drm_sched_job_timedout
[ 1418.269986] PC is at dma_fence_remove_callback+0x8/0x50
[ 1418.275218] LR is at drm_sched_job_timedout+0x4c/0x118
...
[ 1418.415891] [] (dma_fence_remove_callback) from []
(drm_sched_job_timedout+0x4c/0x118)
[ 1418.425571] [] (drm_sched_job_timedout) from []
(process_one_work+0x2c8/0x7bc)
[ 1418.434552] [] (process_one_work) from []
(worker_thread+0x44/0x590)
[ 1418.442663] [] (worker_thread) from []
(kthread+0x160/0x168)
[ 1418.450076] [] (kthread) from []
(ret_from_fork+0x14/0x28)
Cc: Christian König
Cc: Nayan Deshmukh
Cc: Alex Deucher
Signed-off-by: Eric Anholt
---
drivers/gpu/drm/scheduler/sched_main.c | 30 +-
1 file changed, 1 insertion(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index 44fe587aaef9..bd7d11c47202 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -249,41 +249,13 @@ static void drm_sched_job_timedout(struct work_struct
*work)
{
struct drm_gpu_scheduler *sched;
struct drm_sched_job *job;
- int r;
sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
-
- spin_lock(&sched->job_list_lock);
- list_for_each_entry_reverse(job, &sched->ring_mirror_list, node) {
- struct drm_sched_fence *fence = job->s_fence;
-
- if (!dma_fence_remove_callback(fence->parent, &fence->cb))
- goto already_signaled;
- }
-
job = list_first_entry_or_null(&sched->ring_mirror_list,
struct drm_sched_job, node);
- spin_unlock(&sched->job_list_lock);
if (job)
- sched->ops->timedout_job(job);
-
- spin_lock(&sched->job_list_lock);
- list_for_each_entry(job, &sched->ring_mirror_list, node) {
- struct drm_sched_fence *fence = job->s_fence;
-
- if (!fence->parent || !list_empty(&fence->cb.node))
- continue;
-
- r = dma_fence_add_callback(fence->parent, &fence->cb,
- drm_sched_process_job);
- if (r)
- drm_sched_process_job(fence->parent, &fence->cb);
-
-already_signaled:
- ;
- }
- spin_unlock(&sched->job_list_lock);
+ job->sched->ops->timedout_job(job);
}
/**
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] reverts to un-regress v3d
I'm not committed to any of these reverts, as long as the authors can get them fixed. The changes are too intricate for me to make sense of and try to fix myself. Eric Anholt (2): Revert "drm/sched: fix timeout handling v2" drm: Revert syncobj timeline changes. drivers/gpu/drm/drm_syncobj.c | 359 + drivers/gpu/drm/scheduler/sched_main.c | 30 +-- include/drm/drm_syncobj.h | 73 +++-- 3 files changed, 106 insertions(+), 356 deletions(-) -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 107955] AMDGPU driver keeps reloading on hybrid graphics system causing stuttering.
https://bugs.freedesktop.org/show_bug.cgi?id=107955 --- Comment #26 from Ransu --- Sorry for the lack of updates, life got in the way and then this week I made a stupid mistake on where I was sending data with 'dd'. I didn't lose any important data but it did force me to have to setup my laptop from scratch. Good news! As of kernel 4.18.16 I no longer see an issue, knock on wood. I'm going to give it a week and report back but the AMDGPU driver does not seem to be reloading like mad anymore. Linux Y40-80 4.18.16-arch1-1-ARCH #1 SMP PREEMPT Sat Oct 20 22:06:45 UTC 2018 x86_64 GNU/Linux I get the following two messages whenever I request the AMD GPU with "PRIME_DRI=1" and only when I request the AMD GPU. When I'm not making use of the discrete graphics and only the dedicated Intel of my laptop I do not see the below two messages repeated over and over anymore, nor do I see any stuttering. > [drm] PCIE gen 2 link speeds already enabled > amdgpu :05:00.0: PCIE GART of 1024M enabled (table at 0x00F4). One other thing I should note before I set up the system with AMDGPU by putting the following kernel command line arguments into my grub config "radeon.si_support=0 amdgpu.si_support=1"radeon alone would crash my system. I needed to go into a different TTY before login into XFCE to get things setup following this page https://wiki.archlinux.org/index.php/AMDGPU >Nov 08 00:56:06 Y40-80 kernel: radeon :05:00.0: fence driver on ring 4 use >>gpu addr 0x8c10 and cpu addr 0x6a0bf82f >Nov 08 00:56:06 Y40-80 kernel: radeon :05:00.0: fence driver on ring 5 use >>gpu addr 0x00075a18 and cpu addr 0x3d1f62f3 >Nov 08 00:56:06 Y40-80 kernel: radeon :05:00.0: failed VCE resume (-22). >Nov 08 00:56:07 Y40-80 kernel: [drm:r600_ring_test [radeon]] *ERROR* radeon: >>ring 0 test failed (scratch(0x850C)=0xCAFEDEAD) >Nov 08 00:56:07 Y40-80 kernel: [drm:si_resume [radeon]] *ERROR* si startup >>failed on resume >Nov 08 00:56:22 Y40-80 kernel: [drm:atom_op_jump [radeon]] *ERROR* atombios >>stuck in loop for more than 5secs aborting >Nov 08 00:56:22 Y40-80 kernel: [drm:atom_execute_table_locked [radeon]] >*ERROR* >atombios stuck executing C078 (len 237, WS 0, PS 4) @ 0xC086 >Nov 08 00:56:22 Y40-80 kernel: [drm:atom_execute_table_locked [radeon]] >*ERROR* >atombios stuck executing B99E (len 78, WS 12, PS 8) @ 0xB9D7 So now that things appear to be working I just have a few more questions. Does this mean that the discrete GPU should be making use of power saving features and shouldn't be draining too much power if I'm not making use of it? and Does my card support AMDGPU-PRO drivers? If so is there any real advantage of using the "PRO" extras over the standard open source driver? -- 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 201585] 144Hz 2560x1440 no longer works (caps at 120Hz)
https://bugzilla.kernel.org/show_bug.cgi?id=201585 --- Comment #13 from Nicholas Kazlauskas (nicholas.kazlaus...@amd.com) --- (In reply to Dan Acristinii from comment #12) > (In reply to Nicholas Kazlauskas from comment #10) > > (In reply to Nicholas Kazlauskas from comment #9) > > > These patches should fix your issue: > > > > > > https://patchwork.freedesktop.org/series/52164/ > > > > I suppose I should mention that these are for amd-staging-drm-next, they > > probably won't cleanly apply on Linus' tree. > > I would like to mention that revision 2 of the patch is incompatible with > amd-staging-drm-next Are you on the amd-staging-drm-next branch? ie. git checkout --track origin/amd-staging-drm-next -- 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: [PATCH 3/3] drm/vc4: Prefer PPF over TPZ when dst >= 2/3 src
Boris Brezillon writes:
> The HVS spec recommends using PPF when the downscaling ratio is
> between 2/3 and 1. Let's modify vc4_get_scaling_mode() to follow this
> recommendation.
>
> Signed-off-by: Boris Brezillon
> ---
> drivers/gpu/drm/vc4/vc4_plane.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index 5950e6b6b7f0..1d0d91e50aaf 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -129,12 +129,12 @@ static const struct hvs_format *vc4_get_hvs_format(u32
> drm_format)
>
> static enum vc4_scaling_mode vc4_get_scaling_mode(u32 src, u32 dst)
> {
> - if (dst > src)
> + if (dst == src)
> + return VC4_SCALING_NONE;
> + if (3 * dst >= 2 * src)
> return VC4_SCALING_PPF;
> - else if (dst < src)
> - return VC4_SCALING_TPZ;
> else
> - return VC4_SCALING_NONE;
> + return VC4_SCALING_TPZ;
> }
Reviewed-by: Eric Anholt
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/vc4: Force ->x_scaling[1] should never be set to VC4_SCALING_NONE
Boris Brezillon writes:
> On Thu, 08 Nov 2018 06:52:44 -0800
> Eric Anholt wrote:
>
>> Boris Brezillon writes:
>>
>> > For the YUV conversion to work properly, ->x_scaling[0,1] should never
>> > be set to VC4_SCALING_NONE, but vc4_get_scaling_mode() might return
>> > VC4_SCALING_NONE if the horizontal scaling ratio exactly matches the
>> > horizontal subsampling factor. Add a test to turn VC4_SCALING_NONE
>> > into VC4_SCALING_PPF when that happens.
>> >
>> > Fixes: fc04023fafec ("drm/vc4: Add support for YUV planes.")
>> > Signed-off-by: Boris Brezillon
>>
>> I couldn't find a spec justification for this -- did you have a testcase
>> that fails?
>
> Yep. Just set the downscaling ratio to 0.5 with an NV12 format and
> you'll hit the issue (I used modetest to do that):
>
> # modetest -M vc4 -s 29:1920x1080-60 -P 96@95:1920x1080*0.5@NV12
I found that the firmware has a similar behavior to your patch ("if Y is
!unity (x or scaling) and UV is unity, set UV to HPPF/VPPF scaling").
They also select the unity flag after the YUV scaling fixup.
Regardless, if this works, it's got my reviewed-by.
Hopefully we can do some IGT with writeback or chamelium testing all of
the X/Y scaling options with a focus on hitting these 1:1 ratios. The
state space is big and the docs are just ambiguous enough.
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/vc4: Force ->x_scaling[1] should never be set to VC4_SCALING_NONE
On Thu, 08 Nov 2018 06:52:44 -0800
Eric Anholt wrote:
> Boris Brezillon writes:
>
> > For the YUV conversion to work properly, ->x_scaling[0,1] should never
> > be set to VC4_SCALING_NONE, but vc4_get_scaling_mode() might return
> > VC4_SCALING_NONE if the horizontal scaling ratio exactly matches the
> > horizontal subsampling factor. Add a test to turn VC4_SCALING_NONE
> > into VC4_SCALING_PPF when that happens.
> >
> > Fixes: fc04023fafec ("drm/vc4: Add support for YUV planes.")
> > Signed-off-by: Boris Brezillon
>
> I couldn't find a spec justification for this -- did you have a testcase
> that fails?
Yep. Just set the downscaling ratio to 0.5 with an NV12 format and
you'll hit the issue (I used modetest to do that):
# modetest -M vc4 -s 29:1920x1080-60 -P 96@95:1920x1080*0.5@NV12
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/vc4: Force ->x_scaling[1] should never be set to VC4_SCALING_NONE
Boris Brezillon writes:
> For the YUV conversion to work properly, ->x_scaling[0,1] should never
> be set to VC4_SCALING_NONE, but vc4_get_scaling_mode() might return
> VC4_SCALING_NONE if the horizontal scaling ratio exactly matches the
> horizontal subsampling factor. Add a test to turn VC4_SCALING_NONE
> into VC4_SCALING_PPF when that happens.
>
> Fixes: fc04023fafec ("drm/vc4: Add support for YUV planes.")
> Signed-off-by: Boris Brezillon
I couldn't find a spec justification for this -- did you have a testcase
that fails?
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 5/5] drm/amdgpu: Set FreeSync state using drm VRR properties
Support for AMDGPU specific FreeSync properties and ioctls are dropped
from amdgpu_dm in favor of supporting drm variable refresh rate
properties.
The notify_freesync and set_freesync_property functions are dropped
from amdgpu_display_funcs.
The drm vrr_capable property is now attached to any DP/HDMI connector.
Its value is updated accordingly to the connector's FreeSync capabiltiy.
The freesync_enable logic and ioctl control has has been dropped in
favor of utilizing the vrr_enabled on the drm CRTC. This allows for more
fine grained atomic control over which CRTCs should support variable
refresh rate.
To handle state changes for vrr_enabled it was easiest to drop the
forced modeset on freesync_enabled change. This patch now performs the
required stream updates when planes are flipped.
This is done for a few reasons:
(1) VRR stream updates can be done in the fast update path
(2) amdgpu_dm_atomic_check would need to be hacked apart to check
desired variable refresh state and capability before the CRTC
disable pass.
(3) Performing VRR stream updates on-flip is needed for enabling BTR
support.
VRR packets and timing adjustments are now tracked and compared to
previous values sent to the hardware.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Harry Wentland
---
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 7 -
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 255 +-
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 7 +-
3 files changed, 138 insertions(+), 131 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index b9e9e8b02fb7..0cbe867ec375 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -295,13 +295,6 @@ struct amdgpu_display_funcs {
uint16_t connector_object_id,
struct amdgpu_hpd *hpd,
struct amdgpu_router *router);
- /* it is used to enter or exit into free sync mode */
- int (*notify_freesync)(struct drm_device *dev, void *data,
- struct drm_file *filp);
- /* it is used to allow enablement of freesync mode */
- int (*set_freesync_property)(struct drm_connector *connector,
-struct drm_property *property,
-uint64_t val);
};
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index b0df6dc9a775..53eb3d16f75c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1809,72 +1809,6 @@ static void dm_bandwidth_update(struct amdgpu_device
*adev)
/* TODO: implement later */
}
-static int amdgpu_notify_freesync(struct drm_device *dev, void *data,
- struct drm_file *filp)
-{
- struct drm_atomic_state *state;
- struct drm_modeset_acquire_ctx ctx;
- struct drm_crtc *crtc;
- struct drm_connector *connector;
- struct drm_connector_state *old_con_state, *new_con_state;
- int ret = 0;
- uint8_t i;
- bool enable = false;
-
- drm_modeset_acquire_init(&ctx, 0);
-
- state = drm_atomic_state_alloc(dev);
- if (!state) {
- ret = -ENOMEM;
- goto out;
- }
- state->acquire_ctx = &ctx;
-
-retry:
- drm_for_each_crtc(crtc, dev) {
- ret = drm_atomic_add_affected_connectors(state, crtc);
- if (ret)
- goto fail;
-
- /* TODO rework amdgpu_dm_commit_planes so we don't need this */
- ret = drm_atomic_add_affected_planes(state, crtc);
- if (ret)
- goto fail;
- }
-
- for_each_oldnew_connector_in_state(state, connector, old_con_state,
new_con_state, i) {
- struct dm_connector_state *dm_new_con_state =
to_dm_connector_state(new_con_state);
- struct drm_crtc_state *new_crtc_state;
- struct amdgpu_crtc *acrtc =
to_amdgpu_crtc(dm_new_con_state->base.crtc);
- struct dm_crtc_state *dm_new_crtc_state;
-
- if (!acrtc) {
- ASSERT(0);
- continue;
- }
-
- new_crtc_state = drm_atomic_get_new_crtc_state(state,
&acrtc->base);
- dm_new_crtc_state = to_dm_crtc_state(new_crtc_state);
-
- dm_new_crtc_state->freesync_enabled = enable;
- }
-
- ret = drm_atomic_commit(state);
-
-fail:
- if (ret == -EDEADLK) {
- drm_atomic_state_clear(state);
- drm_modeset_backoff(&ctx);
- goto retry;
- }
-
- drm_atomic_state_put(state);
-
-out:
- drm_modeset_drop_locks(&ctx);
- drm_modeset_acquire_fini(&ctx);
- return ret;
-}
static const
[PATCH v7 2/5] drm: Add vrr_enabled property to drm CRTC
This patch introduces the 'vrr_enabled' CRTC property to allow
dynamic control over variable refresh rate support for a CRTC.
This property should be treated like a content hint to the driver -
if the hardware or driver is not capable of driving variable refresh
timings then this is not considered an error.
Capability for variable refresh rate support should be determined
by querying the vrr_capable drm connector property.
It is worth noting that while the property is intended for atomic use
it isn't filtered from legacy userspace queries. This allows for Xorg
userspace drivers to implement support.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Harry Wentland
Cc: Manasi Navare
---
drivers/gpu/drm/drm_atomic_uapi.c | 4
drivers/gpu/drm/drm_crtc.c| 2 ++
drivers/gpu/drm/drm_mode_config.c | 6 ++
include/drm/drm_crtc.h| 9 +
include/drm/drm_mode_config.h | 5 +
5 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index d5b7f315098c..eec396a57b88 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -433,6 +433,8 @@ static int drm_atomic_crtc_set_property(struct drm_crtc
*crtc,
ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
drm_property_blob_put(mode);
return ret;
+ } else if (property == config->prop_vrr_enabled) {
+ state->vrr_enabled = val;
} else if (property == config->degamma_lut_property) {
ret = drm_atomic_replace_property_blob_from_id(dev,
&state->degamma_lut,
@@ -491,6 +493,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = state->active;
else if (property == config->prop_mode_id)
*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+ else if (property == config->prop_vrr_enabled)
+ *val = state->vrr_enabled;
else if (property == config->degamma_lut_property)
*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
else if (property == config->ctm_property)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 268a182ae189..6f8ddfcfaba5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -340,6 +340,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev,
struct drm_crtc *crtc,
drm_object_attach_property(&crtc->base, config->prop_mode_id,
0);
drm_object_attach_property(&crtc->base,
config->prop_out_fence_ptr, 0);
+ drm_object_attach_property(&crtc->base,
+ config->prop_vrr_enabled, 0);
}
return 0;
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index ee80788f2c40..5670c67f28d4 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -310,6 +310,12 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_mode_id = prop;
+ prop = drm_property_create_bool(dev, 0,
+ "VRR_ENABLED");
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_vrr_enabled = prop;
+
prop = drm_property_create(dev,
DRM_MODE_PROP_BLOB,
"DEGAMMA_LUT", 0);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b21437bc95bf..39c3900aab3c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -290,6 +290,15 @@ struct drm_crtc_state {
*/
u32 pageflip_flags;
+ /**
+* @vrr_enabled:
+*
+* Indicates if variable refresh rate should be enabled for the CRTC.
+* Support for the requested vrr state will depend on driver and
+* hardware capabiltiy - lacking support is not treated as failure.
+*/
+ bool vrr_enabled;
+
/**
* @event:
*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 928e4172a0bb..49f2fcfdb5fc 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -639,6 +639,11 @@ struct drm_mode_config {
* connectors must be of and active must be set to disabled, too.
*/
struct drm_property *prop_mode_id;
+ /**
+* @prop_vrr_enabled: Default atomic CRTC property to indicate
+* whether variable refresh rate should be enabled on the CRTC.
+*/
+ struct drm_property *prop_vrr_enabled;
/**
* @dvi_i_subconnector_property: Optional DVI-I property to
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 4/5] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal
When variable refresh rate is active the hardware counter can return
a position >= vtotal. This results in a vpos being returned from
amdgpu_display_get_crtc_scanoutpos that's a positive value. The
positive value indicates to the caller that the display is
currently in scanout when the display is actually still in vblank.
This is because the vfront porch duration is unknown with variable
refresh active and will end when either a page flip occurs or the
timeout specified by the driver/display is reached.
The behavior of the amdgpu_display_get_crtc_scanoutpos remains the
same when the position is below vtotal. When the position is above
vtotal the function will return a value that is effectively -vbl_end,
the size of the vback porch.
The only caller affected by this change is the DRM helper for
calculating vblank timestamps. This change corrects behavior for
calculating the page flip timestamp from being the previous timestamp
to the calculation to the next timestamp when position >= vtotal.
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Harry Wentland
Cc: Michel Dänzer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 6748cd7fc129..cb331319f225 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -850,7 +850,12 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device
*dev,
/* Inside "upper part" of vblank area? Apply corrective offset if so: */
if (in_vbl && (*vpos >= vbl_start)) {
vtotal = mode->crtc_vtotal;
- *vpos = *vpos - vtotal;
+
+ /* With variable refresh rate displays the vpos can exceed
+* the vtotal value. Clamp to 0 to return -vbl_end instead
+* of guessing the remaining number of lines until scanout.
+*/
+ *vpos = (*vpos < vtotal) ? (*vpos - vtotal) : 0;
}
/* Correct for shifted end of vbl at vbl_end. */
--
2.19.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 3/5] drm: Document variable refresh properties
These include the drm_connector 'vrr_capable' and the drm_crtc 'vrr_enabled' properties. Signed-off-by: Nicholas Kazlauskas Cc: Harry Wentland Cc: Manasi Navare Cc: Pekka Paalanen Cc: Ville Syrjälä Cc: Michel Dänzer --- Documentation/gpu/drm-kms.rst | 7 drivers/gpu/drm/drm_connector.c | 68 + 2 files changed, 75 insertions(+) diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 4b1501b4835b..8da2a178cf85 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -575,6 +575,13 @@ Explicit Fencing Properties .. kernel-doc:: drivers/gpu/drm/drm_atomic_uapi.c :doc: explicit fencing properties + +Variable Refresh Properties +--- + +.. kernel-doc:: drivers/gpu/drm/drm_connector.c + :doc: Variable refresh properties + Existing KMS Properties --- diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 49290060ab7b..0e4287461997 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1255,6 +1255,74 @@ int drm_mode_create_scaling_mode_property(struct drm_device *dev) } EXPORT_SYMBOL(drm_mode_create_scaling_mode_property); +/** + * DOC: Variable refresh properties + * + * Variable refresh rate capable displays can dynamically adjust their + * refresh rate by extending the duration of their vertical front porch + * until page flip or timeout occurs. This can reduce or remove stuttering + * and latency in scenarios where the page flip does not align with the + * vblank interval. + * + * An example scenario would be an application flipping at a constant rate + * of 48Hz on a 60Hz display. The page flip will frequently miss the vblank + * interval and the same contents will be displayed twice. This can be + * observed as stuttering for content with motion. + * + * If variable refresh rate was active on a display that supported a + * variable refresh range from 35Hz to 60Hz no stuttering would be observable + * for the example scenario. The minimum supported variable refresh rate of + * 35Hz is below the page flip frequency and the vertical front porch can + * be extended until the page flip occurs. The vblank interval will be + * directly aligned to the page flip rate. + * + * Not all userspace content is suitable for use with variable refresh rate. + * Large and frequent changes in vertical front porch duration may worsen + * perceived stuttering for input sensitive applications. + * + * Panel brightness will also vary with vertical front porch duration. Some + * panels may have noticeable differences in brightness between the minimum + * vertical front porch duration and the maximum vertical front porch duration. + * Large and frequent changes in vertical front porch duration may produce + * observable flickering for such panels. + * + * Userspace control for variable refresh rate is supported via properties + * on the &drm_connector and &drm_crtc objects. + * + * "vrr_capable": + * Optional &drm_connector boolean property that drivers should attach + * with drm_connector_attach_vrr_capable_property() on connectors that + * could support variable refresh rates. Drivers should update the + * property value by calling drm_connector_set_vrr_capable_property(). + * + * Absence of the property should indicate absence of support. + * + * "vrr_enabled": + * Default &drm_crtc boolean property that notifies the driver that the + * content on the CRTC is suitable for variable refresh rate presentation. + * The driver will take this property as a hint to enable variable + * refresh rate support if the receiver supports it, ie. if the + * "vrr_capable" property is true on the &drm_connector object. The + * vertical front porch duration will be extended until page-flip or + * timeout when enabled. + * + * The minimum vertical front porch duration is defined as the vertical + * front porch duration for the current mode. + * + * The maximum vertical front porch duration is greater than or equal to + * the minimum vertical front porch duration. The duration is derived + * from the minimum supported variable refresh rate for the connector. + * + * The driver may place further restrictions within these minimum + * and maximum bounds. + * + * The semantics for the vertical blank timestamp differ when + * variable refresh rate is active. The vertical blank timestamp + * is defined to be an estimate using the current mode's fixed + * refresh rate timings. The semantics for the page-flip event + * timestamp remain the same. + */ + /** * drm_connector_attach_vrr_capable_property - creates the * vrr_capable property -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 0/5] A DRM API for adaptive sync and variable refresh rate support
These patches are part of a proposed new interface for supporting variable refresh rate via DRM properties. === Changes from v6 === drm changes: * Updated documentation typos, added more information about potential issues with luminance changes === Changes from v5 === drm changes: * Updated documentation to define userspace expectations when variable refresh rate is enabled amd changes: * Added patch to fix vblank timestamp calculations when vpos > vtotal === Changes from v4 === amd changes: * Removed unused FreeSync functions from amdgpu === Changes from v3 === drm changes: * Docstring and formatting fixes amd/display changes: * Updated commit message and debug statements === Changes from v2 === The interface has changed substantially from the last revision and the cover letter has been updated accordingly. drm changes: * Most "variable_refresh" prefixes in code have been shortened to just "vrr". This was motivated after changes to the interface had function names close to 80 characters long. Comments from the mailing list were already shortening these in discussion as well. * Documentation for "Variable Refresh" has been added to the KMS properties subsection for DRM driver developers. * The drm_connector property "variable_refresh_capable" has been renamed to "vrr_capable". * The drm_crtc property "variable_refresh" has been been renamed "vrr_enabled" to better reflect its usage. * The drm_connector "variable_refresh_enabled" property has been removed. Managing this is now up to userspace when it sets "vrr_enabled". * The "vrr_capable" property no longer has any state in drm_connector_state associated with it. The value can now be updated with the drm_connector_set_vrr_capable_property() function. This better matches the immutable flag on the property. * The "variable_refresh_changed" flag has been removed from atomic helpers based on feedback from the mailing list and updated interface usage in the amd kernel driver. amd/display changes: * Updated interface usage based on the new properties * Updated VRR infopacket handling based on new xf86-video-amdgpu usage === Adaptive sync and variable refresh rate === Adaptive sync is part of the DisplayPort specification and allows for graphics adapters to drive displays with varying frame timings. Variable refresh rate (VRR) is essentially the same, but defined for HDMI. === Use cases for variable refresh rate === Variable frame (flip) timings don't align well with fixed refresh rate displays. This results in stuttering, tearing and/or input lag. By adjusting the display refresh rate dynamically these issues can be reduced or eliminated. However, not all content is suitable for dynamic refresh adaptation. The user may experience "flickering" from differences in panel luminance at different refresh rates. Content that flips at an infrequent rate or demand is more likely to cause large changes in refresh rate that result in flickering. Userland needs a way to let the driver know when the screen content is suitable for variable refresh rates. === DRM API to support variable refresh rates === This patch introduces a new API via atomic properties on the DRM connector and CRTC. The drm_connector has one new optional boolean property: * bool vrr_capable - set by the driver if the hardware is capable of supporting variable refresh rates The drm_crtc has one new default boolean property: * bool vrr_enabled - set by userspace indicating that variable refresh rate should be enabled == Overview for DRM driver developers === Driver developers can attach the "vrr_capable" property by calling drm_connector_attach_vrr_capable_property(). This should be done on connectors that could be capable of supporting variable refresh rates (such as DP or HDMI). The "vrr_capable" can then be updated accordingly by calling drm_connector_set_vrr_capable_property(). The "vrr_enabled" property can be checked from the drm_crtc_state object. === Overview for Userland developers == The "vrr_enabled" property on the CRTC should be set to true when the CRTC is suitable for variable refresh rates. To demonstrate the suitability of the API for variable refresh and dynamic adaptation there are additional patches using this API that implement adaptive variable refresh across kernel and userland projects: * DRM (dri-devel) * amdgpu DRM kernel driver (amd-gfx) * xf86-video-amdgpu (https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu) * mesa (mesa-dev) These patches enable adaptive variable refresh on X for AMD hardware. Support for variable refresh is disabled by default in xf86-video-amdgpu and will require additional user configuration. The patches have been tested as working on upstream userland with the GNOME desktop environment under a single monitor setup. They also work on KDE in a single monitor setup with the compositor disabled. The patches require that suitable applications flip via the Present extensio
[PATCH v7 1/5] drm: Add vrr_capable property to the drm connector
Modern display hardware is capable of supporting variable refresh rates.
This patch introduces the "vrr_capable" property on the connector to
allow userspace to query support for variable refresh rates.
Atomic drivers should attach this property to connectors that are
capable of driving variable refresh rates using
drm_connector_attach_vrr_capable_property().
The value should be updated based on driver and hardware capability
by using drm_connector_set_vrr_capable_property().
Signed-off-by: Nicholas Kazlauskas
Reviewed-by: Manasi Navare
Reviewed-by: Harry Wentland
---
drivers/gpu/drm/drm_connector.c | 49 +
include/drm/drm_connector.h | 15 ++
2 files changed, 64 insertions(+)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 4943cef178be..49290060ab7b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1255,6 +1255,37 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev)
}
EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
+/**
+ * drm_connector_attach_vrr_capable_property - creates the
+ * vrr_capable property
+ * @connector: connector to create the vrr_capable property on.
+ *
+ * This is used by atomic drivers to add support for querying
+ * variable refresh rate capability for a connector.
+ *
+ * Returns:
+ * Zero on success, negative errono on failure.
+ */
+int drm_connector_attach_vrr_capable_property(
+ struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_property *prop;
+
+ if (!connector->vrr_capable_property) {
+ prop = drm_property_create_bool(dev, DRM_MODE_PROP_IMMUTABLE,
+ "vrr_capable");
+ if (!prop)
+ return -ENOMEM;
+
+ connector->vrr_capable_property = prop;
+ drm_object_attach_property(&connector->base, prop, 0);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_vrr_capable_property);
+
/**
* drm_connector_attach_scaling_mode_property - attach atomic scaling mode
property
* @connector: connector to attach scaling mode property on.
@@ -1583,6 +1614,24 @@ void drm_connector_set_link_status_property(struct
drm_connector *connector,
}
EXPORT_SYMBOL(drm_connector_set_link_status_property);
+/**
+ * drm_connector_set_vrr_capable_property - sets the variable refresh rate
+ * capable property for a connector
+ * @connector: drm connector
+ * @capable: True if the connector is variable refresh rate capable
+ *
+ * Should be used by atomic drivers to update the indicated support for
+ * variable refresh rate over a connector.
+ */
+void drm_connector_set_vrr_capable_property(
+ struct drm_connector *connector, bool capable)
+{
+ drm_object_property_set_value(&connector->base,
+ connector->vrr_capable_property,
+ capable);
+}
+EXPORT_SYMBOL(drm_connector_set_vrr_capable_property);
+
/**
* drm_connector_init_panel_orientation_property -
* initialize the connecters panel_orientation property
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 9ccad6b062f2..4e6befff153b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -959,6 +959,17 @@ struct drm_connector {
*/
struct drm_property *scaling_mode_property;
+ /**
+* @vrr_capable_property: Optional property to help userspace
+* query hardware support for variable refresh rate on a connector.
+* connector. Drivers can add the property to a connector by
+* calling drm_connector_attach_vrr_capable_property().
+*
+* This should be updated only by calling
+* drm_connector_set_vrr_capable_property().
+*/
+ struct drm_property *vrr_capable_property;
+
/**
* @content_protection_property: DRM ENUM property for content
* protection. See drm_connector_attach_content_protection_property().
@@ -1250,6 +1261,8 @@ int drm_mode_create_scaling_mode_property(struct
drm_device *dev);
int drm_connector_attach_content_type_property(struct drm_connector *dev);
int drm_connector_attach_scaling_mode_property(struct drm_connector *connector,
u32 scaling_mode_mask);
+int drm_connector_attach_vrr_capable_property(
+ struct drm_connector *connector);
int drm_connector_attach_content_protection_property(
struct drm_connector *connector);
int drm_mode_create_aspect_ratio_property(struct drm_device *dev);
@@ -1266,6 +1279,8 @@ int drm_connector_update_edid_property(struct
drm_connector *connector,
const struct edid *edid);
void drm_connector_set_link_status_property(struct drm_connector *connector,
uint64_t link_statu
Re: [PATCH] drm/scheduler: Add drm_sched_suspend/resume timeout functions
Am 08.11.18 um 14:42 schrieb Sharat Masetty:
> Hi Christian,
>
> Can you please review this patch? It is a continuation of the discussion at
> [1].
> At first I was thinking of using a cancel for suspend instead of a mod(to an
> arbitrarily large value), but I couldn't get it to fit in as I have an
> additional
> constraint of being able to call these functions from an IRQ context.
>
> These new functions race with other contexts, primarily finish_job(),
> timedout_job() and recovery(), but I did go through the possible races between
> these(I think). Please let me know what you think of this? I have not tested
> this yet and if this is something in the right direction, I will put this
> through my testing drill and polish it.
>
> IMO I think I prefer the callback approach as it appears to be simple, less
> error prone for both the scheduler and the drivers.
Well I agree that this is way to complicated and looks racy to me as
well. But this is because you moved away from my initial suggestion.
So here is once more how to do it without any additional locks or races:
/**
* drm_sched_suspend_timeout - suspend timeout for reset worker
*
* @sched: scheduler instance for which to suspend the timeout
*
* Suspend the delayed work timeout for the scheduler. Note that
* this function can be called from an IRQ context. It returns the
timeout remaining.
*/
unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched)
{
unsigned long timeout, current = jiffies
timeout = sched->work_tdr.timer.expires;
/*
* Set timeout to an arbitrarily large value, this also prevents the
timer to be
* started when new submissions arrive.
*/
if (mod_delayed_work(system_wq, &sched->work_tdr, sched->timeout * 10)
&&
time_after(timeout, current))
return timeout - current;
else
return sched->timeout;
}
/**
* drm_sched_resume_timeout - resume timeout for reset worker
*
* @sched: scheduler instance for which to resume the timeout
* @remaining: remaining timeout
*
* Resume the delayed work timeout for the scheduler. Note that
* this function can be called from an IRQ context.
*/
void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, unsigned long
remaining)
{
if (list_empty(&sched->ring_mirror_list))
cancel_delayed_work(&sched->work_tdr);
else
mod_delayed_work(system_wq, &sched->work_tdr, remaining);
}
Regards,
Christian.
>
> [1] https://patchwork.freedesktop.org/patch/259914/
>
> Signed-off-by: Sharat Masetty
> ---
> drivers/gpu/drm/scheduler/sched_main.c | 81
> +-
> include/drm/gpu_scheduler.h| 5 +++
> 2 files changed, 85 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/scheduler/sched_main.c
> b/drivers/gpu/drm/scheduler/sched_main.c
> index c993d10..9645789 100644
> --- a/drivers/gpu/drm/scheduler/sched_main.c
> +++ b/drivers/gpu/drm/scheduler/sched_main.c
> @@ -191,11 +191,84 @@ bool drm_sched_dependency_optimized(struct dma_fence*
> fence,
>*/
> static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
> +
> + sched->timeout_remaining = sched->timeout;
> +
> if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
> - !list_empty(&sched->ring_mirror_list))
> + !list_empty(&sched->ring_mirror_list) && !sched->work_tdr_suspended)
> schedule_delayed_work(&sched->work_tdr, sched->timeout);
> +
> + spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
> }
>
> +/**
> + * drm_sched_suspend_timeout - suspend timeout for reset worker
> + *
> + * @sched: scheduler instance for which to suspend the timeout
> + *
> + * Suspend the delayed work timeout for the scheduler. Note that
> + * this function can be called from an IRQ context.
> + */
> +void drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched)
> +{
> + unsigned long flags, timeout;
> +
> + spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
> +
> + if (sched->work_tdr_suspended ||
> + sched->timeout == MAX_SCHEDULE_TIMEOUT ||
> + list_empty(&sched->ring_mirror_list))
> + goto done;
> +
> + timeout = sched->work_tdr.timer.expires;
> +
> + /*
> + * Reset timeout to an arbitrarily large value
> + */
> + mod_delayed_work(system_wq, &sched->work_tdr, sched->timeout * 10);
> +
> + timeout -= jiffies;
> +
> + /* FIXME: Can jiffies be after timeout? */
> + sched->timeout_remaining = time_after(jiffies, timeout)? 0: timeout;
> + sched->work_tdr_suspended = true;
> +
> +done:
> + spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
> +}
> +EXPORT_SYMBOL(drm_sched_suspend_timeout);
> +
> +/**
> + * drm_sched_resume_timeout - resume timeout for reset w
[PULL] drm-intel-fixes
Hi Dave, Here's drm-intel-fixes for -rc2. This now includes the GVT fixes too. There's one OOPS fix and memory corruption fix for GVT, as the most important ones. Also a fix for user reported Bugzilla #108282 on 32-bit systems with new Mesa. HDMI 2.0 audio clock mode corrections and removal of unneeded W/As. Other should buy back some perf on Broxton/Geminilake which is not prone to 16G DIMM issue. Then the usual Fixes: tagged material picked by machinery. Regards, Joonas PS. There're some LPSCON dmesg warns in the CI, but I'm assured by display folks I need not worry about them being related to these -fixes even it may seem so. Just planetary de-alignment happened at the same time. *** drm-intel-fixes-2018-11-08: Bugzilla #108282 fixed: Avoid graphics corruption on 32-bit systems for Mesa 18.2.x Avoid OOPS on LPE audio deinit. Remove two unused W/As. Fix to correct HDMI 2.0 audio clock modes to spec. The following changes since commit 651022382c7f8da46cb4872a545ee1da6d097d2a: Linux 4.20-rc1 (2018-11-04 15:37:52 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2018-11-08 for you to fetch changes up to 214782da8fe8497b9af39095c784f3a633e377ec: Merge tag 'gvt-fixes-2018-11-07' of https://github.com/intel/gvt-linux into drm-intel-fixes (2018-11-07 15:34:10 +0200) Bugzilla #108282 fixed: Avoid graphics corruption on 32-bit systems for Mesa 18.2.x Avoid OOPS on LPE audio deinit. Remove two unused W/As. Fix to correct HDMI 2.0 audio clock modes to spec. Chris Wilson (3): drm/i915: Mark up GTT sizes as u64 drm/i915: Compare user's 64b GTT offset even on 32b drm/i915: Mark pin flags as u64 Clint Taylor (1): drm/i915/hdmi: Add HDMI 2.0 audio clock recovery N values Dhinakaran Pandiyan (1): drm/i915: Fix VIDEO_DIP_CTL bit shifts Hang Yuan (2): drm/i915/gvt: invalidate old ggtt page when update ggtt entry drm/i915/gvt: support inconsecutive partial gtt entry write Joonas Lahtinen (1): Merge tag 'gvt-fixes-2018-11-07' of https://github.com/intel/gvt-linux into drm-intel-fixes Longhe Zheng (1): drm/i915/gvt: Handle values of EDP_PSR_IMR and EDP_PSR_IIR Manasi Navare (1): drm/i915/icl: Fix the macros for DFLEXDPMLE register bits Rodrigo Vivi (1): drm/i915/glk: Remove 99% limitation. Ville Syrjälä (4): drm/i915: Don't apply the 16Gb DIMM wm latency w/a to BXT/GLK drm/i915: Fix error handling for the NV12 fb dimensions check drm/i915: Don't oops during modeset shutdown after lpe audio deinit drm/i915: Fix ilk+ watermarks when disabling pipes Xinyun Liu (1): drm/i915/gvt: correct mask setting for CSFE_CHICKEN1 drivers/gpu/drm/i915/gvt/gtt.c| 115 +- drivers/gpu/drm/i915/gvt/gtt.h| 10 ++- drivers/gpu/drm/i915/gvt/handlers.c | 8 +- drivers/gpu/drm/i915/gvt/mmio_context.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 15 ++-- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/i915_gem_execbuffer.c| 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.h | 36 drivers/gpu/drm/i915/i915_reg.h | 20 +++-- drivers/gpu/drm/i915/intel_audio.c| 17 drivers/gpu/drm/i915/intel_cdclk.c| 18 +--- drivers/gpu/drm/i915/intel_display.c | 19 ++--- drivers/gpu/drm/i915/intel_lpe_audio.c| 4 +- drivers/gpu/drm/i915/intel_pm.c | 3 +- drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 +- 17 files changed, 145 insertions(+), 135 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/scheduler: Add drm_sched_suspend/resume timeout functions
Hi Christian,
Can you please review this patch? It is a continuation of the discussion at [1].
At first I was thinking of using a cancel for suspend instead of a mod(to an
arbitrarily large value), but I couldn't get it to fit in as I have an
additional
constraint of being able to call these functions from an IRQ context.
These new functions race with other contexts, primarily finish_job(),
timedout_job() and recovery(), but I did go through the possible races between
these(I think). Please let me know what you think of this? I have not tested
this yet and if this is something in the right direction, I will put this
through my testing drill and polish it.
IMO I think I prefer the callback approach as it appears to be simple, less
error prone for both the scheduler and the drivers.
[1] https://patchwork.freedesktop.org/patch/259914/
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/scheduler/sched_main.c | 81 +-
include/drm/gpu_scheduler.h| 5 +++
2 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/scheduler/sched_main.c
b/drivers/gpu/drm/scheduler/sched_main.c
index c993d10..9645789 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -191,11 +191,84 @@ bool drm_sched_dependency_optimized(struct dma_fence*
fence,
*/
static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
+
+ sched->timeout_remaining = sched->timeout;
+
if (sched->timeout != MAX_SCHEDULE_TIMEOUT &&
- !list_empty(&sched->ring_mirror_list))
+ !list_empty(&sched->ring_mirror_list) && !sched->work_tdr_suspended)
schedule_delayed_work(&sched->work_tdr, sched->timeout);
+
+ spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
}
+/**
+ * drm_sched_suspend_timeout - suspend timeout for reset worker
+ *
+ * @sched: scheduler instance for which to suspend the timeout
+ *
+ * Suspend the delayed work timeout for the scheduler. Note that
+ * this function can be called from an IRQ context.
+ */
+void drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched)
+{
+ unsigned long flags, timeout;
+
+ spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
+
+ if (sched->work_tdr_suspended ||
+ sched->timeout == MAX_SCHEDULE_TIMEOUT ||
+ list_empty(&sched->ring_mirror_list))
+ goto done;
+
+ timeout = sched->work_tdr.timer.expires;
+
+ /*
+* Reset timeout to an arbitrarily large value
+*/
+ mod_delayed_work(system_wq, &sched->work_tdr, sched->timeout * 10);
+
+ timeout -= jiffies;
+
+ /* FIXME: Can jiffies be after timeout? */
+ sched->timeout_remaining = time_after(jiffies, timeout)? 0: timeout;
+ sched->work_tdr_suspended = true;
+
+done:
+ spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
+}
+EXPORT_SYMBOL(drm_sched_suspend_timeout);
+
+/**
+ * drm_sched_resume_timeout - resume timeout for reset worker
+ *
+ * @sched: scheduler instance for which to resume the timeout
+ *
+ * Resume the delayed work timeout for the scheduler. Note that
+ * this function can be called from an IRQ context.
+ */
+void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
+
+ if (!sched->work_tdr_suspended ||
+ sched->timeout == MAX_SCHEDULE_TIMEOUT) {
+ spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
+ return;
+ }
+
+ mod_delayed_work(system_wq, &sched->work_tdr, sched->timeout_remaining);
+
+ sched->work_tdr_suspended = false;
+
+ spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
+}
+EXPORT_SYMBOL(drm_sched_resume_timeout);
+
/* job_finish is called after hw fence signaled
*/
static void drm_sched_job_finish(struct work_struct *work)
@@ -348,6 +421,11 @@ void drm_sched_job_recovery(struct drm_gpu_scheduler
*sched)
struct drm_sched_job *s_job, *tmp;
bool found_guilty = false;
int r;
+ unsigned long flags;
+
+ spin_lock_irqsave(&sched->tdr_suspend_lock, flags);
+ sched->work_tdr_suspended = false;
+ spin_unlock_irqrestore(&sched->tdr_suspend_lock, flags);
spin_lock(&sched->job_list_lock);
list_for_each_entry_safe(s_job, tmp, &sched->ring_mirror_list, node) {
@@ -607,6 +685,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched,
init_waitqueue_head(&sched->job_scheduled);
INIT_LIST_HEAD(&sched->ring_mirror_list);
spin_lock_init(&sched->job_list_lock);
+ spin_lock_init(&sched->tdr_suspend_lock);
atomic_set(&sched->hw_rq_count, 0);
INIT_DELAYED_WORK(&sched->work_tdr, drm_sched_job_timedout);
atomic_set(&sched->num_jobs, 0);
dif
[Bug 108689] new version of intel_gpu_top lost option of output log to stdio or file
https://bugs.freedesktop.org/show_bug.cgi?id=108689 --- Comment #2 from Alex <3.1...@ukr.net> --- 1) Yes, can adapt without any problem. I my case I need render usage and video encode/decode usage data, and memory usage speed/capacity. 2)I know, but this is required extra step to convert/represent data in percent and custom parser. Will be cool if intel_gpu_top just print to stdio raw JSON. -- 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
[PATCH v12 0/2] Add XYUV format support
Introduced new XYUV scan-in format for framebuffer and added support for it to i915(SkyLake+). Stanislav Lisovskiy (2): drm: Introduce new DRM_FORMAT_XYUV drm/i915: Adding YUV444 packed format support for skl+ drivers/gpu/drm/drm_fourcc.c | 1 + drivers/gpu/drm/i915/i915_reg.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 12 drivers/gpu/drm/i915/intel_sprite.c | 3 +++ include/uapi/drm/drm_fourcc.h| 1 + 5 files changed, 18 insertions(+), 1 deletion(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 1/2] drm: Introduce new DRM_FORMAT_XYUV
v5: This is YUV444 packed format same as AYUV, but without alpha,
as supported by i915.
v6: Removed unneeded initializer for new XYUV format.
v7: Added is_yuv field initialization according to latest
drm_fourcc format structure initialization changes.
v8: Edited commit message to be more clear about skl+, renamed
PLANE_CTL_FORMAT_AYUV to PLANE_CTL_FORMAT_XYUV as this format
doesn't support per-pixel alpha. Fixed minor code issues.
v9: Moved DRM format check to proper place in intel_framebuffer_init.
v10: Changed DRM_FORMAT_XYUV to be DRM_FORMAT_XYUV
Reviewed-by: Alexandru Gheorghe
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/drm_fourcc.c | 1 +
include/uapi/drm/drm_fourcc.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 3934527e09dc..965464e550e1 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -225,6 +225,7 @@ const struct drm_format_info *__drm_format_info(u32 format)
{ .format = DRM_FORMAT_UYVY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_VYUY,.depth = 0,
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
{ .format = DRM_FORMAT_AYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true,
.is_yuv = true },
+ { .format = DRM_FORMAT_XYUV,.depth = 0,
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
};
unsigned int i;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 0cd40ebfa1b1..e14aaeb7ad46 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -151,6 +151,7 @@ extern "C" {
#define DRM_FORMAT_VYUYfourcc_code('V', 'Y', 'U', 'Y') /*
[31:0] Y1:Cb0:Y0:Cr0 8:8:8:8 little endian */
#define DRM_FORMAT_AYUVfourcc_code('A', 'Y', 'U', 'V') /*
[31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
+#define DRM_FORMAT_XYUVfourcc_code('X', 'Y', 'U', 'V') /*
[31:0] X:Y:Cb:Cr 8:8:8:8 little endian */
/*
* 2 plane RGB + A
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v12 2/2] drm/i915: Adding YUV444 packed format support for skl+
PLANE_CTL_FORMAT_AYUV is already supported, according to hardware
specification.
v2: Edited commit message, removed redundant whitespaces.
v3: Fixed fallthrough logic for the format switch cases.
v4: Yet again fixed fallthrough logic, to reuse code from other case
labels.
v5: Started to use XYUV instead of AYUV, as we don't use alpha.
v6: Removed unneeded initializer for new XYUV format.
v7: Added scaling support for DRM_FORMAT_XYUV
v8: Edited commit message to be more clear about skl+, renamed
PLANE_CTL_FORMAT_AYUV to PLANE_CTL_FORMAT_XYUV as this format
doesn't support per-pixel alpha. Fixed minor code issues.
v9: Moved DRM format check to proper place in intel_framebuffer_init.
v10: Added missing XYUV format to sprite planes for skl+.
v11: Changed DRM_FORMAT_XYUV to be DRM_FORMAT_XYUV.
v12: Fixed rebase conflicts
Reviewed-by: Ville Syrjälä
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/i915/i915_reg.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 12
drivers/gpu/drm/i915/intel_sprite.c | 3 +++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 470b6fd39c4c..efe897324292 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6500,7 +6500,7 @@ enum {
#define PLANE_CTL_FORMAT_XRGB_2101010(2 << 24)
#define PLANE_CTL_FORMAT_XRGB_ (4 << 24)
#define PLANE_CTL_FORMAT_XRGB_16161616F (6 << 24)
-#define PLANE_CTL_FORMAT_AYUV(8 << 24)
+#define PLANE_CTL_FORMAT_XYUV(8 << 24)
#define PLANE_CTL_FORMAT_INDEXED (12 << 24)
#define PLANE_CTL_FORMAT_RGB_565 (14 << 24)
#define ICL_PLANE_CTL_FORMAT_MASK(0x1f << 23)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b219d5858160..5ee5d04feb93 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2617,6 +2617,8 @@ int skl_format_to_fourcc(int format, bool rgb_order, bool
alpha)
return DRM_FORMAT_RGB565;
case PLANE_CTL_FORMAT_NV12:
return DRM_FORMAT_NV12;
+ case PLANE_CTL_FORMAT_XYUV:
+ return DRM_FORMAT_XYUV;
default:
case PLANE_CTL_FORMAT_XRGB_:
if (rgb_order) {
@@ -3495,6 +3497,8 @@ static u32 skl_plane_ctl_format(uint32_t pixel_format)
return PLANE_CTL_FORMAT_XRGB_2101010;
case DRM_FORMAT_XBGR2101010:
return PLANE_CTL_ORDER_RGBX | PLANE_CTL_FORMAT_XRGB_2101010;
+ case DRM_FORMAT_XYUV:
+ return PLANE_CTL_FORMAT_XYUV;
case DRM_FORMAT_YUYV:
return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_YUYV;
case DRM_FORMAT_YVYU:
@@ -4963,6 +4967,7 @@ static int skl_update_scaler_plane(struct
intel_crtc_state *crtc_state,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
break;
default:
DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format
0x%x\n",
@@ -14513,6 +14518,13 @@ static int intel_framebuffer_init(struct
intel_framebuffer *intel_fb,
goto err;
}
break;
+ case DRM_FORMAT_XYUV:
+ if (INTEL_GEN(dev_priv) < 9) {
+ DRM_DEBUG_KMS("unsupported pixel format: %s\n",
+
drm_get_format_name(mode_cmd->pixel_format, &format_name));
+ goto err;
+ }
+ break;
case DRM_FORMAT_YUYV:
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 370c827294d8..bb50b43b6020 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1638,6 +1638,7 @@ static const uint32_t skl_plane_formats[] = {
DRM_FORMAT_YVYU,
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
+ DRM_FORMAT_XYUV,
};
static const uint32_t skl_planar_formats[] = {
@@ -1654,6 +1655,7 @@ static const uint32_t skl_planar_formats[] = {
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
DRM_FORMAT_NV12,
+ DRM_FORMAT_XYUV,
};
static const uint64_t skl_plane_format_modifiers_noccs[] = {
@@ -1795,6 +1797,7 @@ static bool skl_plane_format_mod_supported(struct
drm_plane *_plane,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_NV12:
+ case DRM_FORMAT_XYUV:
if (modifier == I915_FORMAT_MOD_Yf_TILED)
return true;
/* fall through */
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mai
[Bug 108675] [CI][BAT] The binary pm_rpm excited before executing module-reload
https://bugs.freedesktop.org/show_bug.cgi?id=108675 Lakshmi changed: What|Removed |Added Assignee|dri-devel@lists.freedesktop |petri.latv...@intel.com |.org| Status|NEW |ASSIGNED --- Comment #1 from Lakshmi --- Update: Patch is waiting for comments https://patchwork.freedesktop.org/series/52217/ -- 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 108693] black screen with "drm/amd/display: Do not limit color depth to 8bpc" e03fd3f300f6184c1264186a4c815e93bf658abb >=4.18
https://bugs.freedesktop.org/show_bug.cgi?id=108693 Bug ID: 108693 Summary: black screen with "drm/amd/display: Do not limit color depth to 8bpc" e03fd3f300f6184c1264186a4c815e93bf658abb >=4.18 Product: DRI Version: XOrg git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: major Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: andrey.ara...@nixaid.com Hello, some of the MacBookPro 14,3 users have been struggling with the black screen issue on their systems, I have been bisecting the kernel commits that came to v4.18-rc1 and found that the problem is gone when I revert this commit "drm/amd/display: Do not limit color depth to 8bpc" e03fd3f300f6184c1264186a4c815e93bf658abb I presume that the driver cannot get the display_info.bpc from the screen so it defaults to COLOR_DEPTH_UNDEFINED causing the black screen. Please let me know, https://github.com/torvalds/linux/commit/e03fd3f300f6184c1264186a4c815e93bf658abb Followed-by https://github.com/Dunedan/mbp-2016-linux/issues/73#issuecomment-422397681 Kind Regards, Andrey Arapov P.S. I have been sending an email to Harry Wentland & Alex Deucher on 18 Sept. 2018, but no response followed. -- 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 201585] 144Hz 2560x1440 no longer works (caps at 120Hz)
https://bugzilla.kernel.org/show_bug.cgi?id=201585 --- Comment #12 from Dan Acristinii (d...@acristinii.com) --- (In reply to Nicholas Kazlauskas from comment #10) > (In reply to Nicholas Kazlauskas from comment #9) > > These patches should fix your issue: > > > > https://patchwork.freedesktop.org/series/52164/ > > I suppose I should mention that these are for amd-staging-drm-next, they > probably won't cleanly apply on Linus' tree. I would like to mention that revision 2 of the patch is incompatible with amd-staging-drm-next -- 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 201585] 144Hz 2560x1440 no longer works (caps at 120Hz)
https://bugzilla.kernel.org/show_bug.cgi?id=201585 --- Comment #11 from Dan Acristinii (d...@acristinii.com) --- Created attachment 279377 --> https://bugzilla.kernel.org/attachment.cgi?id=279377&action=edit applied patch to amd-staging-drm-next -- 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: [PATCH v3 3/8] drm/tidss: add new driver for TI Keystone platforms
On 07/11/18 11:56, Daniel Vetter wrote: > General design comment, since you decided to write a new driver: Please > don't stack midlayers like this. I know there's a bunch of other drivers Thanks, I think this makes sense. We'll go back to the drawing board =). Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/vc4: Set PPF scaling when the source image is only vertically scaled
On Wed, 07 Nov 2018 09:08:02 -0800
Eric Anholt wrote:
> Boris Brezillon writes:
>
> > On Wed, 24 Oct 2018 12:05:03 +0200
> > Boris Brezillon wrote:
> >
> >> The source image might be only vertically scaled, and in this case
> >> ->is_unity will be false, but we'd still have to force ->x_scaling[0]
> >> to VC4_SCALING_PPF for YUV conversion to work properly.
> >>
> >> Let's replace the ->is_unity test by->x_scaling[0] == VC4_SCALING_NONE
> >> to cope with that.
> >>
> >> Fixes: 658d8cbd07da ("drm/vc4: Fix the "no scaling" case on multi-planar
> >> YUV formats")
> >> Cc:
> >> Signed-off-by: Boris Brezillon
> >> ---
> >> drivers/gpu/drm/vc4/vc4_plane.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c
> >> b/drivers/gpu/drm/vc4/vc4_plane.c
> >> index 60d5ad19cedd..32b7b9f47c5d 100644
> >> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> >> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> >> @@ -318,7 +318,7 @@ static int vc4_plane_setup_clipping_and_scaling(struct
> >> drm_plane_state *state)
> >> * even on a plane that's otherwise 1:1. Looks like only PPF
> >> * works in that case, so let's pick that one.
> >> */
> >> - if (vc4_state->is_unity)
> >> + if (vc4_state->x_scaling[0] == VC4_SCALING_NONE)
> >>vc4_state->x_scaling[0] = VC4_SCALING_PPF;
> >
> > Actually, I'm not sure about this patch is needed. According to the
> > spec, we should not enable the scaler on the Y channel when ->is_unity
> > is true.
> > I tested it, and it seems to work if we just leave x_scaling[0] to
> > VC4_SCALING_NONE.
>
> Sorry, I've been delaying on this series until I could spend some time
> looking at the spec.
>
> I think you're right, I see no reason to kick us out of is_unity for
> YUV -- you can have is_unity, Y unscaled, and UV scaled.
Okay, so I'll just remove
if (vc4_state->x_scaling[0] == VC4_SCALING_NONE)
vc4_state->x_scaling[0] = VC4_SCALING_PPF;
What about the 2 other patches in this series? Patch 2 is fixing a real
bug, patch 3 is not fixing a bug, but according to the spec PPF is
better when the downscaling ratio is small.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Check if primary mst is null
On Thu, 2018-11-08 at 10:20 +0200, Stanislav Lisovskiy wrote:
> Unfortunately drm_dp_get_mst_branch_device which is called from both
> drm_dp_mst_handle_down_rep and drm_dp_mst_handle_up_rep seem to rely
> on that mgr->mst_primary is not NULL, which seem to be wrong as it
> can be
> cleared with simultaneous mode set, if probing fails or in other
> case.
> mgr->lock mutex doesn't protect against that as it might just get
> assigned to NULL right before, not simultaneously.
>
> There are currently bugs 107738, 108616 bugs which crash in
> drm_dp_get_mst_branch_device, caused by this issue.
>
> v2: Refactored the code, as it was nicely noticed.
> Fixed Bugzilla bug numbers(second was 108616, but not 108816)
> and added links.
>
Hi Lyude Paul,
Thank you for quick review, just poking you
here as requested :)
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108616
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107738
> Signed-off-by: Stanislav Lisovskiy
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 5ff1d79b86c4..0e0df398222d 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1275,6 +1275,9 @@ static struct drm_dp_mst_branch
> *drm_dp_get_mst_branch_device(struct drm_dp_mst_
> mutex_lock(&mgr->lock);
> mstb = mgr->mst_primary;
>
> + if (!mstb)
> + goto out;
> +
> for (i = 0; i < lct - 1; i++) {
> int shift = (i % 2) ? 0 : 4;
> int port_num = (rad[i / 2] >> shift) & 0xf;
--
Best Regards,
Lisovskiy Stanislav
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108689] new version of intel_gpu_top lost option of output log to stdio or file
https://bugs.freedesktop.org/show_bug.cgi?id=108689 Tvrtko Ursulin changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #1 from Tvrtko Ursulin --- Hi, Two questions: New intel_gpu_top also supports a pretty different set of data. So if we added this option - can you adapt to the new data and is it useful for your use case? Secondly, you could get the CSV data for all the stuff new intel_gpu_top shows in a bit more raw form from the standard kernel perf tool. Just one example on render command streamer busyness: perf stat -a -e i915/rcs0-busy/ -I 1000 -x As said, all the data new version shows is available like this, albeit with some required post-procesing like converting absolute time differences to percentages and similar. -- 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] drm/syncobj: disable the timeline UAPI for now
On Thu, Nov 08, 2018 at 09:42:08AM +0100, Christian König wrote:
> Until we have sorted out all problems.
>
> Signed-off-by: Christian König
> ---
> include/drm/drm_syncobj.h | 3 +++
> include/uapi/drm/drm.h| 1 -
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
> index 29244cbcd05e..ffd1f4fcf519 100644
> --- a/include/drm/drm_syncobj.h
> +++ b/include/drm/drm_syncobj.h
> @@ -30,6 +30,9 @@
>
> struct drm_syncobj_cb;
>
> +/* Move the define here for the moment to avoid exposing the UAPI just yet */
> +#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1)
Also needs the EINVAL check in drm_syncobj_create_ioctl() updated. With
that:
Reviewed-by: Daniel Vetter
> +
> enum drm_syncobj_type {
> DRM_SYNCOBJ_TYPE_BINARY,
> DRM_SYNCOBJ_TYPE_TIMELINE
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index cebdb2541eb7..300f336633f2 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -717,7 +717,6 @@ struct drm_prime_handle {
> struct drm_syncobj_create {
> __u32 handle;
> #define DRM_SYNCOBJ_CREATE_SIGNALED (1 << 0)
> -#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1)
> __u32 flags;
> };
>
> --
> 2.17.1
>
> ___
> 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
[PATCH] drm/syncobj: disable the timeline UAPI for now
Until we have sorted out all problems.
Signed-off-by: Christian König
---
include/drm/drm_syncobj.h | 3 +++
include/uapi/drm/drm.h| 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 29244cbcd05e..ffd1f4fcf519 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -30,6 +30,9 @@
struct drm_syncobj_cb;
+/* Move the define here for the moment to avoid exposing the UAPI just yet */
+#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1)
+
enum drm_syncobj_type {
DRM_SYNCOBJ_TYPE_BINARY,
DRM_SYNCOBJ_TYPE_TIMELINE
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index cebdb2541eb7..300f336633f2 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -717,7 +717,6 @@ struct drm_prime_handle {
struct drm_syncobj_create {
__u32 handle;
#define DRM_SYNCOBJ_CREATE_SIGNALED (1 << 0)
-#define DRM_SYNCOBJ_CREATE_TYPE_TIMELINE (1 << 1)
__u32 flags;
};
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108689] new version of intel_gpu_top lost option of output log to stdio or file
https://bugs.freedesktop.org/show_bug.cgi?id=108689 Petri Latvala changed: What|Removed |Added QA Contact|intel-gfx-bugs@lists.freede | |sktop.org | Component|DRM/Intel |IGT Assignee|intel-gfx-bugs@lists.freede |dri-devel@lists.freedesktop |sktop.org |.org -- 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: [PULL] drm-misc-next
Op 07-11-18 om 21:48 schreef Sean Paul: > On Wed, Nov 07, 2018 at 09:31:51PM +0100, Daniel Vetter wrote: >> On Wed, Nov 7, 2018 at 9:29 PM Sean Paul wrote: >>> On Wed, Nov 07, 2018 at 09:18:16PM +0100, Daniel Vetter wrote: On Wed, Nov 07, 2018 at 12:58:56PM +0100, Maarten Lankhorst wrote: > Hey Dave, > > First pull for drm-next this cycle. There's been a lot of changes, so I > hope I recorded everything from the changelog correctly. > > drm-misc-next-2018-11-07: > drm-misc-next for v4.21, part 1: > > UAPI Changes: > - Add syncobj timeline support to drm. With all the CI breakage this caused I kinda missed that it didn't get reverted. But afaict this didn't have the ack from anv/radv folks (which I explicitly asked for as part of what I think should be the merge criteria), and I'm not sure where the userspace is, and this here isn't just prep, but already adds new uapi. >>> +Christian >>> >>> Can you please land the revert while we get this sorted out? >> The revert was for the CI breakage, which is sorted out differently >> already. That was kinda just my excuse for not being in the loop. For >> just the uapi disallowing timeline obj creation and moving the #define >> away from the uapi include is all that's really needed. > Yeah, the uapi #define looked simple enough to back out. Whatever unblocks us > from moving forward is good with me. > > That said, reading through the review thread, this doesn't seem like something > that should have been applied in the first place. I didn't follow the syncobj breakage much, but yeah would be good to have it fixed first. I'll send a new pull req when the revert is applied. :) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 1/2] drm/edid: Add and export function to parse manufacturer id
On Wed, Nov 07, 2018 at 04:23:52PM -0800, José Roberto de Souza wrote:
> This function will be helpful to drivers that wants to add its own
> quirks to sinks.
Why would you want to do that? The point of a shared edid parsing code is
that we can share all these quirks ...
For these kind of patches, always include the driver code that makes use
of your new code too. That makes it much easier to answer these questions.
Thanks, Daniel
>
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/drm_edid.c | 20
> include/drm/drm_edid.h | 1 +
> 2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b506e3622b08..1a0ddf3d326b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1755,6 +1755,21 @@ EXPORT_SYMBOL(drm_edid_duplicate);
>
> /*** EDID parsing ***/
>
> +/**
> + * drm_edid_manufacturer_parse - parse the EDID manufacturer id to readable
> + * characters and set into manufacturer parameter.
> + * @edid: EDID to get the manufacturer
> + * @manufacturer: the char buffer to store the id
> + */
> +void drm_edid_manufacturer_parse(const struct edid *edid, char
> manufacturer[3])
> +{
> + manufacturer[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> + manufacturer[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> + ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> + manufacturer[2] = (edid->mfg_id[1] & 0x1f) + '@';
> +}
> +EXPORT_SYMBOL(drm_edid_manufacturer_parse);
> +
> /**
> * edid_vendor - match a string against EDID's obfuscated vendor field
> * @edid: EDID to match
> @@ -1766,10 +1781,7 @@ static bool edid_vendor(const struct edid *edid, const
> char *vendor)
> {
> char edid_vendor[3];
>
> - edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> - edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> - ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> - edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> + drm_edid_manufacturer_parse(edid, edid_vendor);
>
> return !strncmp(edid_vendor, vendor, 3);
> }
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index e3c404833115..e4f3f7f34d6a 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -466,6 +466,7 @@ struct edid *drm_get_edid_switcheroo(struct drm_connector
> *connector,
>struct i2c_adapter *adapter);
> struct edid *drm_edid_duplicate(const struct edid *edid);
> int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid);
> +void drm_edid_manufacturer_parse(const struct edid *edid, char
> manufacturer[3]);
>
> u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
> enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
> --
> 2.19.1
>
> ___
> 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] drm: Revert "add syncobj timeline support v9"
On Thu, Nov 8, 2018 at 9:12 AM Christian König
wrote:
>
> Am 07.11.18 um 21:33 schrieb Daniel Vetter:
> > On Fri, Oct 19, 2018 at 11:29:49AM +0200, Daniel Vetter wrote:
> >> On Fri, Oct 19, 2018 at 11:27:11AM +0200, Christian König wrote:
> >>> From: Christian König
> >>>
> >>> Still contains some bugs.
> >>>
> >>> This reverts commit 48197bc564c7a1888c86024a1ba4f956e0ec2300.
> >> Thanks for the super-quick handling, much appreciated.
> >>
> >>> Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=108490
> >>> Signed-off-by: Christian König
> >> Acked-by: Daniel Vetter
> > Just quick question: Why did we not push the revert right away, but
> > instead spend a few more days letting CI crash all over for a real fix?
> > Seems a bit backwards to me ...
>
> Well to find more bugs. I mean that's what we have all this unit testing
> for.
The testing is so you can see what goes up in flames before you push,
not afterwards. And imo when it has blown up in obviuos ways that test
would have caught (the igts fully run on amdgpu too, or should at
least), then it's better to restart properly. There's a lot more
people using the upstream trees than just you&Chunming for developing
timeline syncobj.
-Daniel
> It turned out to be the right decision because it not only pointed out
> some more problems in the patch itself, but also not handled corner
> cases in drivers as well as a problem in dma-fence-array.
>
> Except for one false positive warning from lockdep all of those should
> be fixed by now and Chunming is already working on that last item.
>
> BTW: I've intentionally holding back most of the UAPI because we haven't
> got any anv/radv reviews for that. The only UAPI currently exposed is
> the DRM_SYNCOBJ_CREATE_TYPE_TIMELINE flag.
>
> Christian.
>
> > -Daniel
> >
> >> Cheers, Daniel
> >>
> >>> ---
> >>> drivers/gpu/drm/drm_syncobj.c | 306 +++--
> >>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> >>> include/drm/drm_syncobj.h | 67 +++--
> >>> include/uapi/drm/drm.h | 1 -
> >>> 4 files changed, 75 insertions(+), 301 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >>> index 57bf6006394d..e2c5b3ca4824 100644
> >>> --- a/drivers/gpu/drm/drm_syncobj.c
> >>> +++ b/drivers/gpu/drm/drm_syncobj.c
> >>> @@ -56,9 +56,6 @@
> >>> #include "drm_internal.h"
> >>> #include
> >>>
> >>> -/* merge normal syncobj to timeline syncobj, the point interval is 1 */
> >>> -#define DRM_SYNCOBJ_BINARY_POINT 1
> >>> -
> >>> struct drm_syncobj_stub_fence {
> >>> struct dma_fence base;
> >>> spinlock_t lock;
> >>> @@ -74,11 +71,6 @@ static const struct dma_fence_ops
> >>> drm_syncobj_stub_fence_ops = {
> >>> .get_timeline_name = drm_syncobj_stub_fence_get_name,
> >>> };
> >>>
> >>> -struct drm_syncobj_signal_pt {
> >>> - struct dma_fence_array *fence_array;
> >>> - u64value;
> >>> - struct list_head list;
> >>> -};
> >>>
> >>> /**
> >>>* drm_syncobj_find - lookup and reference a sync object.
> >>> @@ -121,8 +113,8 @@ static int
> >>> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> >>> {
> >>> int ret;
> >>>
> >>> - ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
> >>> - if (!ret)
> >>> + *fence = drm_syncobj_fence_get(syncobj);
> >>> + if (*fence)
> >>> return 1;
> >>>
> >>> spin_lock(&syncobj->lock);
> >>> @@ -130,12 +122,10 @@ static int
> >>> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> >>> * have the lock, try one more time just to be sure we don't add a
> >>> * callback when a fence has already been set.
> >>> */
> >>> - if (!list_empty(&syncobj->signal_pt_list)) {
> >>> - spin_unlock(&syncobj->lock);
> >>> - drm_syncobj_search_fence(syncobj, 0, 0, fence);
> >>> - if (*fence)
> >>> - return 1;
> >>> - spin_lock(&syncobj->lock);
> >>> + if (syncobj->fence) {
> >>> + *fence =
> >>> dma_fence_get(rcu_dereference_protected(syncobj->fence,
> >>> +
> >>> lockdep_is_held(&syncobj->lock)));
> >>> + ret = 1;
> >>> } else {
> >>> *fence = NULL;
> >>> drm_syncobj_add_callback_locked(syncobj, cb, func);
> >>> @@ -163,160 +153,6 @@ void drm_syncobj_remove_callback(struct drm_syncobj
> >>> *syncobj,
> >>> spin_unlock(&syncobj->lock);
> >>> }
> >>>
> >>> -static void drm_syncobj_init(struct drm_syncobj *syncobj)
> >>> -{
> >>> - spin_lock(&syncobj->lock);
> >>> - syncobj->timeline_context = dma_fence_context_alloc(1);
> >>> - syncobj->timeline = 0;
> >>> - syncobj->signal_point = 0;
> >>> - init_waitqueue_head(&syncobj->wq);
> >>> -
> >>> - INIT_LIST_HEAD(&syncobj->signal_pt_list);
> >>> - spin_unlock(&syncobj->lock);
> >>> -}
> >>> -
> >>> -static void drm_syncobj_fini(struct drm_syncobj *s
[PATCH v2] drm: Check if primary mst is null
Unfortunately drm_dp_get_mst_branch_device which is called from both
drm_dp_mst_handle_down_rep and drm_dp_mst_handle_up_rep seem to rely
on that mgr->mst_primary is not NULL, which seem to be wrong as it can be
cleared with simultaneous mode set, if probing fails or in other case.
mgr->lock mutex doesn't protect against that as it might just get
assigned to NULL right before, not simultaneously.
There are currently bugs 107738, 108616 bugs which crash in
drm_dp_get_mst_branch_device, caused by this issue.
v2: Refactored the code, as it was nicely noticed.
Fixed Bugzilla bug numbers(second was 108616, but not 108816)
and added links.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108616
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107738
Signed-off-by: Stanislav Lisovskiy
---
drivers/gpu/drm/drm_dp_mst_topology.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 5ff1d79b86c4..0e0df398222d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1275,6 +1275,9 @@ static struct drm_dp_mst_branch
*drm_dp_get_mst_branch_device(struct drm_dp_mst_
mutex_lock(&mgr->lock);
mstb = mgr->mst_primary;
+ if (!mstb)
+ goto out;
+
for (i = 0; i < lct - 1; i++) {
int shift = (i % 2) ? 0 : 4;
int port_num = (rad[i / 2] >> shift) & 0xf;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/ttm: remove set but not used variable 'driver'
Am 08.11.18 um 03:15 schrieb YueHaibing:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/gpu/drm/ttm/ttm_execbuf_util.c: In function
> 'ttm_eu_fence_buffer_objects':
> drivers/gpu/drm/ttm/ttm_execbuf_util.c:190:24: warning:
> variable 'driver' set but not used [-Wunused-but-set-variable]
>
> It not used any more after
> commit f2c24b83ae90 ("drm/ttm: flip the switch, and convert to dma_fence")
>
> Signed-off-by: YueHaibing
Reviewed-by: Christian König and going to
push it into our internal branch for upstreaming.
> ---
> drivers/gpu/drm/ttm/ttm_execbuf_util.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index e493edb..efa005a 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -187,14 +187,12 @@ void ttm_eu_fence_buffer_objects(struct ww_acquire_ctx
> *ticket,
> struct ttm_buffer_object *bo;
> struct ttm_bo_global *glob;
> struct ttm_bo_device *bdev;
> - struct ttm_bo_driver *driver;
>
> if (list_empty(list))
> return;
>
> bo = list_first_entry(list, struct ttm_validate_buffer, head)->bo;
> bdev = bo->bdev;
> - driver = bdev->driver;
> glob = bo->bdev->glob;
>
> spin_lock(&glob->lru_lock);
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Revert "add syncobj timeline support v9"
Am 07.11.18 um 21:33 schrieb Daniel Vetter:
On Fri, Oct 19, 2018 at 11:29:49AM +0200, Daniel Vetter wrote:
On Fri, Oct 19, 2018 at 11:27:11AM +0200, Christian König wrote:
From: Christian König
Still contains some bugs.
This reverts commit 48197bc564c7a1888c86024a1ba4f956e0ec2300.
Thanks for the super-quick handling, much appreciated.
Bugs: https://bugs.freedesktop.org/show_bug.cgi?id=108490
Signed-off-by: Christian König
Acked-by: Daniel Vetter
Just quick question: Why did we not push the revert right away, but
instead spend a few more days letting CI crash all over for a real fix?
Seems a bit backwards to me ...
Well to find more bugs. I mean that's what we have all this unit testing
for.
It turned out to be the right decision because it not only pointed out
some more problems in the patch itself, but also not handled corner
cases in drivers as well as a problem in dma-fence-array.
Except for one false positive warning from lockdep all of those should
be fixed by now and Chunming is already working on that last item.
BTW: I've intentionally holding back most of the UAPI because we haven't
got any anv/radv reviews for that. The only UAPI currently exposed is
the DRM_SYNCOBJ_CREATE_TYPE_TIMELINE flag.
Christian.
-Daniel
Cheers, Daniel
---
drivers/gpu/drm/drm_syncobj.c | 306 +++--
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
include/drm/drm_syncobj.h | 67 +++--
include/uapi/drm/drm.h | 1 -
4 files changed, 75 insertions(+), 301 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 57bf6006394d..e2c5b3ca4824 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,9 +56,6 @@
#include "drm_internal.h"
#include
-/* merge normal syncobj to timeline syncobj, the point interval is 1 */
-#define DRM_SYNCOBJ_BINARY_POINT 1
-
struct drm_syncobj_stub_fence {
struct dma_fence base;
spinlock_t lock;
@@ -74,11 +71,6 @@ static const struct dma_fence_ops drm_syncobj_stub_fence_ops
= {
.get_timeline_name = drm_syncobj_stub_fence_get_name,
};
-struct drm_syncobj_signal_pt {
- struct dma_fence_array *fence_array;
- u64value;
- struct list_head list;
-};
/**
* drm_syncobj_find - lookup and reference a sync object.
@@ -121,8 +113,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
{
int ret;
- ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
- if (!ret)
+ *fence = drm_syncobj_fence_get(syncobj);
+ if (*fence)
return 1;
spin_lock(&syncobj->lock);
@@ -130,12 +122,10 @@ static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
* have the lock, try one more time just to be sure we don't add a
* callback when a fence has already been set.
*/
- if (!list_empty(&syncobj->signal_pt_list)) {
- spin_unlock(&syncobj->lock);
- drm_syncobj_search_fence(syncobj, 0, 0, fence);
- if (*fence)
- return 1;
- spin_lock(&syncobj->lock);
+ if (syncobj->fence) {
+ *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
+
lockdep_is_held(&syncobj->lock)));
+ ret = 1;
} else {
*fence = NULL;
drm_syncobj_add_callback_locked(syncobj, cb, func);
@@ -163,160 +153,6 @@ void drm_syncobj_remove_callback(struct drm_syncobj
*syncobj,
spin_unlock(&syncobj->lock);
}
-static void drm_syncobj_init(struct drm_syncobj *syncobj)
-{
- spin_lock(&syncobj->lock);
- syncobj->timeline_context = dma_fence_context_alloc(1);
- syncobj->timeline = 0;
- syncobj->signal_point = 0;
- init_waitqueue_head(&syncobj->wq);
-
- INIT_LIST_HEAD(&syncobj->signal_pt_list);
- spin_unlock(&syncobj->lock);
-}
-
-static void drm_syncobj_fini(struct drm_syncobj *syncobj)
-{
- struct drm_syncobj_signal_pt *signal_pt = NULL, *tmp;
-
- spin_lock(&syncobj->lock);
- list_for_each_entry_safe(signal_pt, tmp,
-&syncobj->signal_pt_list, list) {
- list_del(&signal_pt->list);
- dma_fence_put(&signal_pt->fence_array->base);
- kfree(signal_pt);
- }
- spin_unlock(&syncobj->lock);
-}
-
-static struct dma_fence
-*drm_syncobj_find_signal_pt_for_point(struct drm_syncobj *syncobj,
- uint64_t point)
-{
- struct drm_syncobj_signal_pt *signal_pt;
-
- if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) &&
- (point <= syncobj->timeline)) {
- struct drm_syncobj_stub_fence *fence =
- kzalloc(sizeof(struct drm_syncobj_stub_fence),
-
Re: [PULL] drm-misc-next
On Thu, Nov 8, 2018 at 8:56 AM Christian König wrote: > > Am 07.11.18 um 21:48 schrieb Sean Paul: > > On Wed, Nov 07, 2018 at 09:31:51PM +0100, Daniel Vetter wrote: > >> On Wed, Nov 7, 2018 at 9:29 PM Sean Paul wrote: > >>> On Wed, Nov 07, 2018 at 09:18:16PM +0100, Daniel Vetter wrote: > On Wed, Nov 07, 2018 at 12:58:56PM +0100, Maarten Lankhorst wrote: > > Hey Dave, > > > > First pull for drm-next this cycle. There's been a lot of changes, so I > > hope I recorded everything from the changelog correctly. > > > > drm-misc-next-2018-11-07: > > drm-misc-next for v4.21, part 1: > > > > UAPI Changes: > > - Add syncobj timeline support to drm. > With all the CI breakage this caused I kinda missed that it didn't get > reverted. But afaict this didn't have the ack from anv/radv folks (which > I > explicitly asked for as part of what I think should be the merge > criteria), and I'm not sure where the userspace is, and this here isn't > just prep, but already adds new uapi. > > >>> +Christian > >>> > >>> Can you please land the revert while we get this sorted out? > >> The revert was for the CI breakage, which is sorted out differently > >> already. That was kinda just my excuse for not being in the loop. For > >> just the uapi disallowing timeline obj creation and moving the #define > >> away from the uapi include is all that's really needed. > > Yeah, the uapi #define looked simple enough to back out. Whatever unblocks > > us > > from moving forward is good with me. > > Agreed, should I take care of that? Sounds good to me, I'll be happy to do a quick review. -Daniel > > > > > That said, reading through the review thread, this doesn't seem like > > something > > that should have been applied in the first place. > > Well it was the usual problem of not getting enough attention on a patch > set. > > Christian. > > > > > Sean > > > >> Assuming of course that I'm not again having my stuff all mixed up :-) > >> -Daniel > >> > >>> Sean > >>> > The prep work is imo totally ok to keep, but the uapi side seems rather > questionable. > -Daniel > > > Cross-subsystem Changes: > > - Remove shared fence staging in dma-buf's fence object, and allow > >reserving more than 1 fence and add more paranoia when debugging. > > - Constify infoframe functions in video/hdmi. > > > > Core Changes: > > - Add vkms todo, and a lot of assorted doc fixes. > > - Drop transitional helpers and convert drivers to use > > drm_atomic_helper_shutdown(). > > - Move atomic state helper functions to drm_atomic_state_helper.[ch] > > - Refactor drm selftests, and add new tests. > > - DP MST atomic state cleanups. > > - Drop EXPORT_SYMBOL from drm leases. > > - Lease cleanups and fixes. > > - Create render node for vgem. > > > > Driver Changes: > > - Fix build failure in imx without fbdev emulation. > > - Add rotation quirk for GPD win2 panel. > > - Add support for various CDTech panels, Banana Pi Panel, DLC1010GIG, > >Olimex LCD-O-LinuXino, Samsung S6D16D0, Truly NT35597 WQXGA, > >Himax HX8357D, simulated RTSM AEMv8. > > - Add dw_hdmi support to rockchip driver. > > - Fix YUV support in vc4. > > - Fix resource id handling in virtio. > > - Make rockchip use dw-mipi-dsi bridge driver, and add dual dsi support. > > - Advertise that tinydrm only supports DRM_FORMAT_MOD_LINEAR. > > - Convert many drivers to use atomic helpers, and > > drm_fbdev_generic_setup(). > > - Add Mali linear tiled formats, and enable them in the Mali-DP driver. > > - Add support for H6 DE3 mixer 0, DW HDMI, HDMI PHY and TCON TOP. > > - Assorted driver cleanups and fixes. > > > > The following changes since commit > > f2bfc71aee75feff33ca659322b72ffeed5a243d: > > > >Merge tag 'drm-intel-next-fixes-2018-10-18' of > > git://anongit.freedesktop.org/drm/drm-intel into drm-next (2018-10-19 > > 14:28:11 +1000) > > > > are available in the Git repository at: > > > >git://anongit.freedesktop.org/drm/drm-misc > > tags/drm-misc-next-2018-11-07 > > > > for you to fetch changes up to e7afb623b4fb82089c9a50c733c740522b8220bc: > > > >drm: Add drm_any_plane_has_format() (2018-11-06 21:34:22 +0200) > > > > > > drm-misc-next for v4.21, part 1: > > > > UAPI Changes: > > - Add syncobj timeline support to drm. > > > > Cross-subsystem Changes: > > - Remove shared fence staging in dma-buf's fence object, and allow > >reserving more than 1 fence and add more paranoia when debugging. > > - Constify infoframe functions in video/hdmi. > > > > Core Changes: > > - Add vkms todo, and a lot of assorted doc fixes. > > - Drop transitional helpers and convert drivers to
