[Bug 203325] 5.1 amdgpu on polaris11: broken text-fb multihead, works in X

2019-04-18 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=203325

Duncan (1i5t5.dun...@cox.net) changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |CODE_FIX

--- Comment #5 from Duncan (1i5t5.dun...@cox.net) ---
(In reply to Alex Deucher from comment #4)
> Should be fixed with this patch:
> https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-fixes-5.
> 1&id=c238bfe0be9ef7420f7669a69e27c8c8f4d8a568

Thanks.  That's in mainline now, merged just today, and does indeed appear to
fix it. =:^)

I'm confused between code_fix and patch_already_available, and the status-help
link doesn't help because those are apparently kernel-bugzilla-specific and the
documentation isn't so doesn't cover them (bug filed on that years ago,
but...), so I'll guess code_fix.

-- 
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: Support for 2D engines/blitters in V4L2 and DRM

2019-04-18 Thread Tomasz Figa
On Fri, Apr 19, 2019 at 9:30 AM Nicolas Dufresne  wrote:
>
> Le jeudi 18 avril 2019 à 10:18 +0200, Daniel Vetter a écrit :
> > > It would be cool if both could be used concurrently and not just return
> > > -EBUSY when the device is used with the other subsystem.
> >
> > We live in this world already :-) I think there's even patches (or merged
> > already) to add fences to v4l, for Android.
>
> This work is currently suspended. It will require some feature on DRM
> display to really make this useful, but there is also a lot of
> challanges in V4L2. In GFX space, most of the use case are about
> rendering as soon as possible. Though, in multimedia we have two
> problems, we need to synchronize the frame rendering with the audio,
> and output buffers may comes out of order due to how video CODECs are
> made.
>
> In the first, we'd need a mechanism where we can schedule a render at a
> specific time or vblank. We can of course already implement this in
> software, but with fences, the scheduling would need to be done in the
> driver. Then if the fence is signalled earlier, the driver should hold
> on until the delay is met. If the fence got signalled late, we also
> need to think of a workflow. As we can't schedule more then one render
> in DRM at one time, I don't really see yet how to make that work.
>
> For the second, it's complicated on V4L2 side. Currently we signal
> buffers when they are ready in the display order. With fences, we
> receive early pairs buffer and fence (in decoding order). There exist
> cases where reordering is done by the driver (stateful CODEC). We
> cannot schedule these immediately we would need a new mechanism to know
> which one come next. If we just reuse current mechnism, it would void
> the fence usage since the fence will always be signalled by the time it
> reaches DRM or other v4l2 component.
>
> There also other issues, for video capture pipeline, if you are not
> rendering ASAP, you need the HW timestamp in order to schedule. Again,
> we'd get the fence early, but the actual timestamp will be signalled at
> the very last minutes, so we also risk of turning the fence into pure
> overhead. Note that as we speak, I have colleagues who are
> experimenting with frame timestamp prediction that slaves to the
> effective timestamp (catching up over time). But we still have issues
> when the capture driver skipped a frame (missed a capture window).

Note that a fence has a timestamp internally and it can be queried for
it from the user space if exposed as a sync file:
https://elixir.bootlin.com/linux/v5.1-rc5/source/drivers/dma-buf/sync_file.c#L386

Fences in V4L2 would be also useful for stateless decoders and any
mem-to-mem processors that operate in order, like the blitters
mentioned here or actually camera ISPs, which can be often chained
into relatively sophisticated pipelines.

Best regards,
Tomasz
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 01/29] tracing: Cleanup stack trace code

2019-04-18 Thread Steven Rostedt
On Fri, 19 Apr 2019 00:44:17 +0200 (CEST)
Thomas Gleixner  wrote:

> On Thu, 18 Apr 2019, Steven Rostedt wrote:
> > On Thu, 18 Apr 2019 10:41:20 +0200
> > Thomas Gleixner  wrote:
> >   
> > > @@ -412,23 +404,20 @@ stack_trace_sysctl(struct ctl_table *tab
> > >  void __user *buffer, size_t *lenp,
> > >  loff_t *ppos)
> > >  {
> > > - int ret;
> > > + int ret, was_enabled;  
> > 
> > One small nit. Could this be:
> > 
> > int was_enabled;
> > int ret;
> > 
> > I prefer only joining variables that are related on the same line.
> > Makes it look cleaner IMO.  
> 
> If you wish so. To me it's waste of screen space :)

At least you didn't say it helps the compiler ;-)

> 
> > >  
> > >   mutex_lock(&stack_sysctl_mutex);
> > > + was_enabled = !!stack_tracer_enabled;
> > >
> > 
> > Bah, not sure why I didn't do it this way to begin with. I think I
> > copied something else that couldn't do it this way for some reason and
> > didn't put any brain power behind the copy. :-/ But that was back in
> > 2008 so I blame it on being "young and stupid" ;-)  
> 
> The young part is gone for sure :)

I purposely set you up for that response.

> 
> > Other then the above nit and removing the unneeded +1 in max_entries:  
> 
> s/+1/-1/

That was an ode to G+

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/v3d: Set the correct DMA mask according to the MMU's limits.

2019-04-18 Thread Eric Anholt
On 7278, we've got 40 bits to work with.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 1 +
 drivers/gpu/drm/v3d/v3d_drv.c | 6 +-
 drivers/gpu/drm/v3d/v3d_regs.h| 8 
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c 
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 356a8acfa72d..ab652a034959 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -30,6 +30,7 @@ static const struct v3d_reg_def v3d_hub_reg_defs[] = {
REGDEF(V3D_MMU_CTL),
REGDEF(V3D_MMU_VIO_ADDR),
REGDEF(V3D_MMU_VIO_ID),
+   REGDEF(V3D_MMU_DEBUG_INFO),
 };
 
 static const struct v3d_reg_def v3d_gca_reg_defs[] = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index f8d1d2569c1f..7ab36192e6bc 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -472,9 +472,9 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
struct drm_device *drm;
struct v3d_dev *v3d;
int ret;
+   u32 mmu_debug;
u32 ident1;
 
-   dev->coherent_dma_mask = DMA_BIT_MASK(36);
 
v3d = kzalloc(sizeof(*v3d), GFP_KERNEL);
if (!v3d)
@@ -491,6 +491,10 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
if (ret)
goto dev_free;
 
+   mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
+   dev->coherent_dma_mask =
+   DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
+
ident1 = V3D_READ(V3D_HUB_IDENT1);
v3d->ver = (V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_TVER) * 10 +
V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_REV));
diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
index 9a8ff0ce648e..54c8c4320da0 100644
--- a/drivers/gpu/drm/v3d/v3d_regs.h
+++ b/drivers/gpu/drm/v3d/v3d_regs.h
@@ -191,6 +191,14 @@
 /* Address that faulted */
 #define V3D_MMU_VIO_ADDR   0x01234
 
+#define V3D_MMU_DEBUG_INFO 0x01238
+# define V3D_MMU_PA_WIDTH_MASK V3D_MASK(11, 8)
+# define V3D_MMU_PA_WIDTH_SHIFT8
+# define V3D_MMU_VA_WIDTH_MASK V3D_MASK(7, 4)
+# define V3D_MMU_VA_WIDTH_SHIFT4
+# define V3D_MMU_VERSION_MASK  V3D_MASK(3, 0)
+# define V3D_MMU_VERSION_SHIFT 0
+
 /* Per-V3D-core registers */
 
 #define V3D_CTL_IDENT0 0x0
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/v3d: Dump V3D error debug registers in debugfs, and one at reset.

2019-04-18 Thread Eric Anholt
Looking at a hang recently, I noticed these registers that might tell
me if something obvious was wrong.  They didn't help in this case, but
keep it around for the future.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_debugfs.c |  5 
 drivers/gpu/drm/v3d/v3d_gem.c |  4 +++-
 drivers/gpu/drm/v3d/v3d_regs.h| 38 +++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c 
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index ab652a034959..78a78938e81f 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -58,6 +58,11 @@ static const struct v3d_reg_def v3d_core_reg_defs[] = {
REGDEF(V3D_GMP_STATUS),
REGDEF(V3D_GMP_CFG),
REGDEF(V3D_GMP_VIO_ADDR),
+
+   REGDEF(V3D_ERR_FDBGO),
+   REGDEF(V3D_ERR_FDBGB),
+   REGDEF(V3D_ERR_FDBGS),
+   REGDEF(V3D_ERR_STAT),
 };
 
 static const struct v3d_reg_def v3d_csd_reg_defs[] = {
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index f736e021467a..27e0f87075d9 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -109,7 +109,9 @@ v3d_reset(struct v3d_dev *v3d)
 {
struct drm_device *dev = &v3d->drm;
 
-   DRM_ERROR("Resetting GPU.\n");
+   DRM_DEV_ERROR(dev->dev, "Resetting GPU for hang.\n");
+   DRM_DEV_ERROR(dev->dev, "V3D_ERR_STAT: 0x%08x\n",
+ V3D_CORE_READ(0, V3D_ERR_STAT));
trace_v3d_reset_begin(dev);
 
/* XXX: only needed for safe powerdown, not reset. */
diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
index 54c8c4320da0..eda1e289976f 100644
--- a/drivers/gpu/drm/v3d/v3d_regs.h
+++ b/drivers/gpu/drm/v3d/v3d_regs.h
@@ -455,4 +455,42 @@
 # define V3D_CSD_CURRENT_ID0_WG_Y_MASK V3D_MASK(15, 0)
 # define V3D_CSD_CURRENT_ID0_WG_Y_SHIFT0
 
+#define V3D_ERR_FDBGO  0x00f04
+#define V3D_ERR_FDBGB  0x00f08
+#define V3D_ERR_FDBGR  0x00f0c
+
+#define V3D_ERR_FDBGS  0x00f10
+# define V3D_ERR_FDBGS_INTERPZ_IP_STALLBIT(17)
+# define V3D_ERR_FDBGS_DEPTHO_FIFO_IP_STALLBIT(16)
+# define V3D_ERR_FDBGS_XYNRM_IP_STALL  BIT(14)
+# define V3D_ERR_FDBGS_EZREQ_FIFO_OP_VALID BIT(13)
+# define V3D_ERR_FDBGS_QXYF_FIFO_OP_VALID  BIT(12)
+# define V3D_ERR_FDBGS_QXYF_FIFO_OP_LAST   BIT(11)
+# define V3D_ERR_FDBGS_EZTEST_ANYQVALIDBIT(7)
+# define V3D_ERR_FDBGS_EZTEST_PASS BIT(6)
+# define V3D_ERR_FDBGS_EZTEST_QREADY   BIT(5)
+# define V3D_ERR_FDBGS_EZTEST_VLF_OKNOVALIDBIT(4)
+# define V3D_ERR_FDBGS_EZTEST_QSTALL   BIT(3)
+# define V3D_ERR_FDBGS_EZTEST_IP_VLFSTALL  BIT(2)
+# define V3D_ERR_FDBGS_EZTEST_IP_PRSTALL   BIT(1)
+# define V3D_ERR_FDBGS_EZTEST_IP_QSTALLBIT(0)
+
+#define V3D_ERR_STAT   0x00f20
+# define V3D_ERR_L2CAREBIT(15)
+# define V3D_ERR_VCMBE BIT(14)
+# define V3D_ERR_VCMRE BIT(13)
+# define V3D_ERR_VCDI  BIT(12)
+# define V3D_ERR_VCDE  BIT(11)
+# define V3D_ERR_VDWE  BIT(10)
+# define V3D_ERR_VPMEASBIT(9)
+# define V3D_ERR_VPMEFNA   BIT(8)
+# define V3D_ERR_VPMEWNA   BIT(7)
+# define V3D_ERR_VPMERNA   BIT(6)
+# define V3D_ERR_VPMERRBIT(5)
+# define V3D_ERR_VPMEWRBIT(4)
+# define V3D_ERR_VPAERRGL  BIT(3)
+# define V3D_ERR_VPAEBRGL  BIT(2)
+# define V3D_ERR_VPAERGS   BIT(1)
+# define V3D_ERR_VPAEABB   BIT(0)
+
 #endif /* V3D_REGS_H */
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/v3d: Fix debugfs reads of MMU regs.

2019-04-18 Thread Eric Anholt
They're in the hub, not the individual cores.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c 
b/drivers/gpu/drm/v3d/v3d_debugfs.c
index a2dc4262955e..356a8acfa72d 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -26,6 +26,10 @@ static const struct v3d_reg_def v3d_hub_reg_defs[] = {
REGDEF(V3D_HUB_IDENT3),
REGDEF(V3D_HUB_INT_STS),
REGDEF(V3D_HUB_INT_MSK_STS),
+
+   REGDEF(V3D_MMU_CTL),
+   REGDEF(V3D_MMU_VIO_ADDR),
+   REGDEF(V3D_MMU_VIO_ID),
 };
 
 static const struct v3d_reg_def v3d_gca_reg_defs[] = {
@@ -50,9 +54,6 @@ static const struct v3d_reg_def v3d_core_reg_defs[] = {
REGDEF(V3D_PTB_BPCA),
REGDEF(V3D_PTB_BPCS),
 
-   REGDEF(V3D_MMU_CTL),
-   REGDEF(V3D_MMU_VIO_ADDR),
-
REGDEF(V3D_GMP_STATUS),
REGDEF(V3D_GMP_CFG),
REGDEF(V3D_GMP_VIO_ADDR),
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/v3d: Fix and extend MMU error handling.

2019-04-18 Thread Eric Anholt
We were setting the wrong flags to enable PTI errors, so we were
seeing reads to invalid PTEs show up as write errors.  Also, we
weren't turning on the interrupts.  The AXI IDs we were dumping
included the outstanding write number and so they looked basically
random.  And the VIO_ADDR decoding was based on the MMU VA_WIDTH for
the first platform I worked on and was wrong on others.  In short,
this was a thorough mess from early HW enabling.

Tested on V3D 4.1 and 4.2 with intentional L2T, CLE, PTB, and TLB
faults.

Signed-off-by: Eric Anholt 
---
 drivers/gpu/drm/v3d/v3d_drv.c  |  1 +
 drivers/gpu/drm/v3d/v3d_drv.h  |  2 ++
 drivers/gpu/drm/v3d/v3d_irq.c  | 31 +++
 drivers/gpu/drm/v3d/v3d_mmu.c  |  7 +--
 drivers/gpu/drm/v3d/v3d_regs.h |  3 ++-
 5 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 7ab36192e6bc..9ce2e4ef6c2a 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -494,6 +494,7 @@ static int v3d_platform_drm_probe(struct platform_device 
*pdev)
mmu_debug = V3D_READ(V3D_MMU_DEBUG_INFO);
dev->coherent_dma_mask =
DMA_BIT_MASK(30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_PA_WIDTH));
+   v3d->va_width = 30 + V3D_GET_FIELD(mmu_debug, V3D_MMU_VA_WIDTH);
 
ident1 = V3D_READ(V3D_HUB_IDENT1);
v3d->ver = (V3D_GET_FIELD(ident1, V3D_HUB_IDENT1_TVER) * 10 +
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 6d31a6a5a08e..64682923018d 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -63,6 +63,8 @@ struct v3d_dev {
 */
void *mmu_scratch;
dma_addr_t mmu_scratch_paddr;
+   /* virtual address bits from V3D to the MMU. */
+   int va_width;
 
/* Number of V3D cores. */
u32 cores;
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index fac3c542860b..268d8a889ac5 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -162,10 +162,33 @@ v3d_hub_irq(int irq, void *arg)
  V3D_HUB_INT_MMU_PTI |
  V3D_HUB_INT_MMU_CAP)) {
u32 axi_id = V3D_READ(V3D_MMU_VIO_ID);
-   u64 vio_addr = (u64)V3D_READ(V3D_MMU_VIO_ADDR) << 8;
-
-   dev_err(v3d->dev, "MMU error from client %d at 
0x%08llx%s%s%s\n",
-   axi_id, (long long)vio_addr,
+   u64 vio_addr = ((u64)V3D_READ(V3D_MMU_VIO_ADDR) <<
+   (v3d->va_width - 32));
+   static const char *const v3d41_axi_ids[] = {
+   "L2T",
+   "PTB",
+   "PSE",
+   "TLB",
+   "CLE",
+   "TFU",
+   "MMU",
+   "GMP",
+   };
+   const char *client = "?";
+
+   V3D_WRITE(V3D_MMU_CTL,
+ V3D_READ(V3D_MMU_CTL) & (V3D_MMU_CTL_CAP_EXCEEDED |
+  V3D_MMU_CTL_PT_INVALID |
+  
V3D_MMU_CTL_WRITE_VIOLATION));
+
+   if (v3d->ver >= 41) {
+   axi_id = axi_id >> 5;
+   if (axi_id < ARRAY_SIZE(v3d41_axi_ids))
+   client = v3d41_axi_ids[axi_id];
+   }
+
+   dev_err(v3d->dev, "MMU error from client %s (%d) at 
0x%llx%s%s%s\n",
+   client, axi_id, (long long)vio_addr,
((intsts & V3D_HUB_INT_MMU_WRV) ?
 ", write violation" : ""),
((intsts & V3D_HUB_INT_MMU_PTI) ?
diff --git a/drivers/gpu/drm/v3d/v3d_mmu.c b/drivers/gpu/drm/v3d/v3d_mmu.c
index 7a21f1787ab1..395e81d97163 100644
--- a/drivers/gpu/drm/v3d/v3d_mmu.c
+++ b/drivers/gpu/drm/v3d/v3d_mmu.c
@@ -69,10 +69,13 @@ int v3d_mmu_set_page_table(struct v3d_dev *v3d)
V3D_WRITE(V3D_MMU_PT_PA_BASE, v3d->pt_paddr >> V3D_MMU_PAGE_SHIFT);
V3D_WRITE(V3D_MMU_CTL,
  V3D_MMU_CTL_ENABLE |
- V3D_MMU_CTL_PT_INVALID |
+ V3D_MMU_CTL_PT_INVALID_ENABLE |
  V3D_MMU_CTL_PT_INVALID_ABORT |
+ V3D_MMU_CTL_PT_INVALID_INT |
  V3D_MMU_CTL_WRITE_VIOLATION_ABORT |
- V3D_MMU_CTL_CAP_EXCEEDED_ABORT);
+ V3D_MMU_CTL_WRITE_VIOLATION_INT |
+ V3D_MMU_CTL_CAP_EXCEEDED_ABORT |
+ V3D_MMU_CTL_CAP_EXCEEDED_INT);
V3D_WRITE(V3D_MMU_ILLEGAL_ADDR,
  (v3d->mmu_scratch_paddr >> V3D_MMU_PAGE_SHIFT) |
  V3D_MMU_ILLEGAL_ADDR_ENABLE);
diff --git a/drivers/gpu/drm/v3d/v3d_regs.h b/drivers/gpu/drm/v3d/v3d_regs.h
index eda1e289976f..9bcb57781d31 100644
--- a/drivers/gpu/drm/v3d/v3d_regs.h
+++ b/drivers/gpu/drm/v3d/v3d_regs.h

[Bug 110214] radeonsi: xterm scrollback buffer disappears while Shift+PgUp and Shift+PgDn

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110214

--- Comment #82 from Diego Viola  ---
I found that I can't reproduce this bug with Xephyr -glamor_gles2 (git) but it
still happens with -glamor.

-- 
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 01/29] tracing: Cleanup stack trace code

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 10:41:20 +0200
Thomas Gleixner  wrote:


> @@ -412,23 +404,20 @@ stack_trace_sysctl(struct ctl_table *tab
>  void __user *buffer, size_t *lenp,
>  loff_t *ppos)
>  {
> - int ret;
> + int ret, was_enabled;

One small nit. Could this be:

int was_enabled;
int ret;

I prefer only joining variables that are related on the same line.
Makes it look cleaner IMO.

>  
>   mutex_lock(&stack_sysctl_mutex);
> + was_enabled = !!stack_tracer_enabled;
>  

Bah, not sure why I didn't do it this way to begin with. I think I
copied something else that couldn't do it this way for some reason and
didn't put any brain power behind the copy. :-/ But that was back in
2008 so I blame it on being "young and stupid" ;-)

Other then the above nit and removing the unneeded +1 in max_entries:

Reviewed-by: Steven Rostedt (VMware) 

-- Steve


>   ret = proc_dointvec(table, write, buffer, lenp, ppos);
>  
> - if (ret || !write ||
> - (last_stack_tracer_enabled == !!stack_tracer_enabled))
> + if (ret || !write || (was_enabled == !!stack_tracer_enabled))
>   goto out;
>  
> - last_stack_tracer_enabled = !!stack_tracer_enabled;
> -
>   if (stack_tracer_enabled)
>   register_ftrace_function(&trace_ops);
>   else
>   unregister_ftrace_function(&trace_ops);
> -
>   out:
>   mutex_unlock(&stack_sysctl_mutex);
>   return ret;
> @@ -444,7 +433,6 @@ static __init int enable_stacktrace(char
>   strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE);
>  
>   stack_tracer_enabled = 1;
> - last_stack_tracer_enabled = 1;
>   return 1;
>  }
>  __setup("stacktrace", enable_stacktrace);
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 01/29] tracing: Cleanup stack trace code

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 17:24:43 -0400
Steven Rostedt  wrote:

> I believe it was for historical leftovers (there was a time it was
> required), and left there for "paranoid" sake. But let me apply the
> patch and see if it is really needed.

I removed the +1 on the max_entries and set SET_TRACE_ENTRIES to 5 (a
bit extreme). Then I ran the stack tracing with KASAN enabled and it
never complained.

As stated, it was there for historical reasons and I felt 500 was way
more than enough and left the buffer there just out of laziness and
paranoia.

Feel free to remove that if you want.

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/nouveau/fb/ramgk104: fix spelling mistake "sucessfully" -> "successfully"

2019-04-18 Thread Mukesh Ojha


On 4/18/2019 10:23 PM, Colin King wrote:

From: Colin Ian King 

There is a spelling mistake in a nvkm_debug message. Fix it.

Signed-off-by: Colin Ian King 

Reviewed-by: Mukesh Ojha 

Cheers,
-Mukesh

---
  drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
index 8bcb7e79a0cb..456aed1f2a02 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
@@ -1070,7 +1070,7 @@ gk104_ram_calc_xits(struct gk104_ram *ram, struct 
nvkm_ram_data *next)
nvkm_error(subdev, "unable to calc plls\n");
return -EINVAL;
}
-   nvkm_debug(subdev, "sucessfully calced PLLs for clock %i kHz"
+   nvkm_debug(subdev, "successfully calced PLLs for clock %i kHz"
" (refclock: %i kHz)\n", next->freq, ret);
} else {
/* calculate refpll coefficients */

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 01/29] tracing: Cleanup stack trace code

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 23:14:45 +0200 (CEST)
Thomas Gleixner  wrote:

> On Thu, 18 Apr 2019, Josh Poimboeuf wrote:
> 
> > On Thu, Apr 18, 2019 at 10:41:20AM +0200, Thomas Gleixner wrote:  
> > > - Remove the extra array member of stack_dump_trace[]. It's not required 
> > > as
> > >   the stack tracer stores at max array size - 1 entries so there is still
> > >   an empty slot.  
> > 
> > What is the empty slot used for?  
> 
> I was trying to find an answer but failed. Maybe it's just historical
> leftovers or Steven knows where the magic is in this maze.
>

I believe it was for historical leftovers (there was a time it was
required), and left there for "paranoid" sake. But let me apply the
patch and see if it is really needed.

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/20] drm: Split out the formats API and move it to a common place

2019-04-18 Thread Maxime Ripard
On Thu, Apr 18, 2019 at 02:32:14PM +0200, Daniel Vetter wrote:
> > > > > Unifying the formats themselves, and all the associated metadata is
> > > > > imo a no-go, and was a pretty conscious decision when we implemented
> > > > > drm_fourcc a few years back.
> > > > >
> > > > > > If we want to keep the current library in DRM, we have two options
> > > > > > then:
> > > > > >
> > > > > >   - Support all the v4l2 formats in the DRM library, which is
> > > > > > essentially what I'm doing in the last patches. However, that
> > > > > > would require to have the v4l2 developpers also reviewing stuff
> > > > > > there. And given how busy they are, I cannot really see how that
> > > > > > would work.
> > > > >
> > > > > Well, if we end up with a common library then yes we need cross
> > > > > review. There's no way around that. Doesn't matter where exactly that
> > > > > library is in the filesystem tree, and adding a special MAINTAINERS
> > > > > entry for anything related to fourcc (both drm and v4l) to make sure
> > > > > they get cross-posted is easy. No file renaming needed.
> > > >
> > > > That would create some governing issues as well. For example, if you
> > > > ever have a patch from one fourcc addition (that might or might not be
> > > > covered by v4l2), will you wait for any v4l2 developper to review it?
> > >
> > > None of this is fixed by code renaming or locations. Either way we
> > > need to figure that out.
> > >
> > > And yes part of the reasons for not moving this out of drm is that I'm
> > > not a fan of boutique trees for small stuff. If sharing means we need
> > > to split the drm_fourcc code and library out of drm trees, I'm not
> > > sure that's a good idea. We're already super liberal with merging
> > > anything through driver trees with acks, and integrating them quickly
> > > into drm-next. This would still be workable if v4l sends regular pull
> > > requests to drm-next (every 1-2 weeks, like the other big gpu trees
> > > do). If we can only sync up once per merge window with a shared
> > > boutique tree for formats only, life is going to be painful.
> >
> > I don't get why we want to turn DRM into some kind of a black hole
> > that would pull everything. We don't have to, really. And at the same
> > time it carries the message that v4l2 is less important than DRM for
> > some reason, which I'm really not comfortable with.
>
> Make another tree somewhere that pulls in trees more often than every
> merge window, and I'm happy. It's the coordination effort of lots of
> trees that creates the black hole, not the other way round. Yes topic
> trees work, but if topic trees are persistent something with the
> organization of trees is wrong and needs to change. This very much
> looks like we'll end up with a perpetual topic branch for format stuff
> between drm and v4l.

Well, if v4l2 sends a PR to DRM every 1 or 2 weeks, that definitely
looks like a topic branch to me. And on a far more frequent basis than
when we will merge a format description.

> The other shared stuff (like hdmi infoframes) seems to change a lot
> less often, so the occasional patch hasn't been a pain. But drm_fourcc
> related stuff sees a lot of work, both in adding new formats and in
> refactoring the library to keep up with all the new use-cases.

When was the last time a refactoring that changed the API happened?

Most of the changes will be new format descriptions, and I guess that
would only concern a single tree.

And really, we're doing this all the time, so I'm not sure what the
big deal is here.

I feel like there's something that you don't really like about this,
but you're not saying this out loud.

Sure, the exact process needs to be figured out, and everyone needs to
agree upon that process. But that's pretty much it, and it's nothing
out of the ordinary.

> And yes I think an overall gfx-like-stuff.git tree for drm and v4l and
> the few other bits really makes tons of sense. Not as a tree where
> people commit, but as the 2nd-level integration tree (like drm.git
> right now for gpu stuff).
>
> > And I don't really get why you're against this in the first
> > place. When you have some code in a single driver that would benefit
> > more driver, you create a helper and move it into the core.
>
> It's a feature that drm doesn't share that much code with other parts
> of the kernel, it makes backporting the gfx stack to lts kernels a lot
> easier. Until someone fixes the upstream kernel release model to no
> longer need large scale gpu driver backports, we need to keep that.
>
> > In this case, we have some code used by a framework that more
> > framework could use, and we move it to a core-er place. How is that
> > different?
>
> Imo core sharing for code sharing's sake is overrated. If we already
> have drm and v4l tightly integrated as a community, then code sharing
> becomes a lot easier, and a lot more reasonable to do.

At least Laurent, Boris, Ezequiel, Gustavo and I have been working on

Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 14:58:55 -0500
Tom Zanussi  wrote:

> > Tom,
> > 
> > Can you review this too?  
> 
> Looks good to me too!
> 
> Acked-by: Tom Zanussi 
> 

Would you be OK to upgrade this to a Reviewed-by tag?

Thanks!

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110469] R5 M330 GPU Hung

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110469

--- Comment #1 from Slava  ---
Created attachment 144038
  --> https://bugs.freedesktop.org/attachment.cgi?id=144038&action=edit
xorg log

-- 
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 110469] R5 M330 GPU Hung

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110469

Bug ID: 110469
   Summary: R5 M330 GPU Hung
   Product: DRI
   Version: XOrg git
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: DRM/Radeon
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: masterxa...@gmail.com

Created attachment 144037
  --> https://bugs.freedesktop.org/attachment.cgi?id=144037&action=edit
dmesg log

When using discrete radeon gpu on my laptop in most application(via PRIME) I
constantly get gpu hung.

01:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Sun XT
[Radeon HD 8670A/8670M/8690M / R5 M330 / M430 / R7 M520] (rev 83)
Subsystem: Hewlett-Packard Company Sun XT [Radeon HD 8670A/8670M/8690M
/ R5 M330 / M430 / Radeon 520 Mobile]
Physical Slot: 0
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- 
Kernel driver in use: radeon
Kernel modules: radeon, amdgpu

I can reproduce this bug in glmark2. bug ALWAYS happens on [desktop] stage of
benchmark. also this is not hardware problem(gpu works fine in windows/ no
overheating)
Please help me, I cant use discrete card for a year now because this bug.

There is call trace in dmesg somehere in the middle(full logs attached):

Apr 18 23:45:40 HP kernel: radeon :01:00.0: ring 3 stalled for more than
238640msec
Apr 18 23:45:40 HP kernel: radeon :01:00.0: GPU lockup (current fence id
0x000257d3 last fence id 0x000257d8 on ring 3)
Apr 18 23:45:40 HP kernel: radeon :01:00.0: ring 0 stalled for more than
239107msec
Apr 18 23:45:40 HP kernel: radeon :01:00.0: GPU lockup (current fence id
0x00012b81 last fence id 0x00012b84 on ring 0)
Apr 18 23:45:40 HP kernel: INFO: task kworker/u8:0:3018 blocked for more than
120 seconds.
Apr 18 23:45:40 HP kernel:   Tainted: G   OE 5.0.3-pf #1
Apr 18 23:45:40 HP kernel: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
Apr 18 23:45:40 HP kernel: kworker/u8:0D0  3018  2 0x8080
Apr 18 23:45:40 HP kernel: Workqueue: events_unbound commit_work
[drm_kms_helper]
Apr 18 23:45:40 HP kernel: Call Trace:
Apr 18 23:45:40 HP kernel:  ? __schedule+0x505/0x14d0
Apr 18 23:45:40 HP kernel:  ? ieee802_11_parse_elems_crc+0x16a/0x650 [mac80211]
Apr 18 23:45:40 HP kernel:  schedule+0x28/0x90
Apr 18 23:45:40 HP kernel:  schedule_timeout+0x23d/0x2e0
Apr 18 23:45:40 HP kernel:  ?
dce110_timing_generator_get_crtc_scanoutpos+0x88/0x110 [amdgpu]
Apr 18 23:45:40 HP kernel:  dma_fence_default_wait+0x204/0x270
Apr 18 23:45:40 HP kernel:  ? dma_fence_wait_timeout+0x100/0x100
Apr 18 23:45:40 HP kernel:  dma_fence_wait_timeout+0xd9/0x100
Apr 18 23:45:40 HP kernel:  reservation_object_wait_timeout_rcu+0x1f2/0x370
Apr 18 23:45:40 HP kernel:  amdgpu_dm_do_flip+0x14a/0x4a0 [amdgpu]
Apr 18 23:45:40 HP kernel:  ? amdgpu_dm_atomic_commit_tail+0x5f9/0xbc0 [amdgpu]
Apr 18 23:45:40 HP kernel:  amdgpu_dm_atomic_commit_tail+0x5f9/0xbc0 [amdgpu]
Apr 18 23:45:40 HP kernel:  commit_tail+0x3d/0x70 [drm_kms_helper]
Apr 18 23:45:40 HP kernel:  process_one_work+0x1f4/0x3f0
Apr 18 23:45:40 HP kernel:  worker_thread+0x2d/0x3e0
Apr 18 23:45:40 HP kernel:  ? process_one_work+0x3f0/0x3f0
Apr 18 23:45:40 HP kernel:  kthread+0x112/0x130
Apr 18 23:45:40 HP kernel:  ? kthread_park+0x80/0x80
Apr 18 23:45:40 HP kernel:  ret_from_fork+0x1f/0x40

-- 
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 107612] [Vega10] Hard Lock [gfxhub] VMC page fault when opening Mario Kart 8 in Cemu under wine

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=107612

--- Comment #4 from bibitocarlos  ---
I would like to help with this bug. I get it with Raven ridge 2400G.
All you have to do is play Mariokart 8 with CEMU.

-- 
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 2/4] drm/amd/display: Fix and simplify apply_below_the_range()

2019-04-18 Thread Kazlauskas, Nicholas
On 4/17/19 11:51 PM, Mario Kleiner wrote:
> The comparison of inserted_frame_duration_in_us against a
> duration calculated from max_refresh_in_uhz is both wrong
> in its math and not needed, as the min_duration_in_us value
> is already cached in in_out_vrr for reuse. No need to
> recalculate it wrongly at each invocation.
> 
> Signed-off-by: Mario Kleiner 

Reviewed-by: Nicholas Kazlauskas 

Looks reasonable to me.

> ---
>   drivers/gpu/drm/amd/display/modules/freesync/freesync.c | 6 ++
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c 
> b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> index 71274683da04..e56543c36eba 100644
> --- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> +++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> @@ -437,10 +437,8 @@ static void apply_below_the_range(struct core_freesync 
> *core_freesync,
>   inserted_frame_duration_in_us = last_render_time_in_us /
>   frames_to_insert;
>   
> - if (inserted_frame_duration_in_us <
> - (100 / in_out_vrr->max_refresh_in_uhz))
> - inserted_frame_duration_in_us =
> - (100 / in_out_vrr->max_refresh_in_uhz);
> + if (inserted_frame_duration_in_us < 
> in_out_vrr->min_duration_in_us)
> + inserted_frame_duration_in_us = 
> in_out_vrr->min_duration_in_us;
>   
>   /* Cache the calculated variables */
>   in_out_vrr->btr.inserted_duration_in_us =
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/amd/display: Add some debug output for VRR BTR.

2019-04-18 Thread Kazlauskas, Nicholas
On 4/17/19 11:51 PM, Mario Kleiner wrote:
> Helps with debugging issues with low framerate compensation.
> 
> Signed-off-by: Mario Kleiner 
> ---

This looks like it'd generate a ton of debug output (the flip stuff is 
already a bit too spammy).

But more importantly the DC and module code doesn't touch the debug 
prints directly.

DC has the DC_LOG_* defines but the modules shouldn't touch anything 
logger related.

Nicholas Kazlauskas

>   .../amd/display/modules/freesync/freesync.c| 18 ++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c 
> b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> index 3d867e34f8b3..71274683da04 100644
> --- a/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> +++ b/drivers/gpu/drm/amd/display/modules/freesync/freesync.c
> @@ -1041,6 +1041,11 @@ void mod_freesync_handle_preflip(struct mod_freesync 
> *mod_freesync,
>   average_render_time_in_us += last_render_time_in_us;
>   average_render_time_in_us /= DC_PLANE_UPDATE_TIMES_MAX;
>   
> + DRM_DEBUG_DRIVER("vrr flip: avg %d us, last %d us, max %d us\n",
> +  average_render_time_in_us,
> +  last_render_time_in_us,
> +  in_out_vrr->max_duration_in_us);
> +
>   if (in_out_vrr->btr.btr_enabled) {
>   apply_below_the_range(core_freesync,
>   stream,
> @@ -1053,6 +1058,10 @@ void mod_freesync_handle_preflip(struct mod_freesync 
> *mod_freesync,
>   in_out_vrr);
>   }
>   
> + DRM_DEBUG_DRIVER("vrr btr_active:%d - num %d of dur %d us\n",
> +  in_out_vrr->btr.btr_active,
> +  in_out_vrr->btr.frames_to_insert,
> +  in_out_vrr->btr.inserted_duration_in_us);
>   }
>   }
>   
> @@ -1090,11 +1099,17 @@ void mod_freesync_handle_v_update(struct mod_freesync 
> *mod_freesync,
>   in_out_vrr->btr.inserted_duration_in_us);
>   in_out_vrr->adjust.v_total_max =
>   in_out_vrr->adjust.v_total_min;
> + DRM_DEBUG_DRIVER("btr start: c=%d, vtotal=%d\n",
> +  in_out_vrr->btr.frames_to_insert,
> +  in_out_vrr->adjust.v_total_min);
>   }
>   
>   if (in_out_vrr->btr.frame_counter > 0)
>   in_out_vrr->btr.frame_counter--;
>   
> + DRM_DEBUG_DRIVER("btr upd: count %d\n",
> +  in_out_vrr->btr.frame_counter);
> +
>   /* Restore FreeSync */
>   if (in_out_vrr->btr.frame_counter == 0) {
>   in_out_vrr->adjust.v_total_min =
> @@ -1103,6 +1118,9 @@ void mod_freesync_handle_v_update(struct mod_freesync 
> *mod_freesync,
>   in_out_vrr->adjust.v_total_max =
>   calc_v_total_from_refresh(stream,
>   in_out_vrr->min_refresh_in_uhz);
> + DRM_DEBUG_DRIVER("btr end: vtotal_min=%d/max=%d\n",
> +  in_out_vrr->adjust.v_total_min,
> +  in_out_vrr->adjust.v_total_max);
>   }
>   }
>   
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110355] radeonsi: GTK elements become invisible in some applications (GIMP, LibreOffice)

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110355

--- Comment #11 from Diego Viola  ---
(In reply to Marek Olšák from comment #10)
> The bug has nothing to do with compute. The compute blit removed gfx-based
> blits from the gfx pipeline, which uncovered a bug in the driver.

Hi Marek,

I tested your patches from this series:

https://patchwork.freedesktop.org/patch/300465/?series=59674&rev=1

and my issue is fixed after applying your patches, thank you. :D

-- 
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 v5 0/3] backlight: lm3630a: bug fix and fwnode support

2019-04-18 Thread Brian Masney
Here is a patch series that fixes a single bug and adds fwnode support
to the lm3630a driver. This work was tested on a LG Nexus 5 (hammerhead)
phone. My status page at https://masneyb.github.io/nexus-5-upstream/
describes what is working so far with the upstream kernel.

See the individual patches for the changelog.

Brian Masney (3):
  backlight: lm3630a: return 0 on success in update_status functions
  dt-bindings: backlight: add lm3630a bindings
  backlight: lm3630a: add firmware node support

 .../leds/backlight/lm3630a-backlight.yaml | 129 +++
 drivers/video/backlight/lm3630a_bl.c  | 153 +-
 include/linux/platform_data/lm3630a_bl.h  |   4 +
 3 files changed, 279 insertions(+), 7 deletions(-)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml

-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/amdgpu: fix spelling mistake "recieve" -> "receive"

2019-04-18 Thread Mukesh Ojha


On 4/18/2019 3:55 PM, Colin King wrote:

From: Colin Ian King 

There is a spelling mistake in a pr_err message. Fix it.

Signed-off-by: Colin Ian King 

Reviewed-by: Mukesh Ojha 

Cheers,
-Mukesh

---
  drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
index 6a0fcd67662a..aef9d059ae52 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_vi.c
@@ -515,7 +515,7 @@ static void xgpu_vi_mailbox_flr_work(struct work_struct 
*work)
  
  	/* wait until RCV_MSG become 3 */

if (xgpu_vi_poll_msg(adev, IDH_FLR_NOTIFICATION_CMPL)) {
-   pr_err("failed to recieve FLR_CMPL\n");
+   pr_err("failed to receive FLR_CMPL\n");
return;
}
  

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 1/3] backlight: lm3630a: return 0 on success in update_status functions

2019-04-18 Thread Brian Masney
lm3630a_bank_a_update_status() and lm3630a_bank_b_update_status()
both return the brightness value if the brightness was successfully
updated. Writing to these attributes via sysfs would cause a 'Bad
address' error to be returned. These functions should return 0 on
success, so let's change it to correct that error.

Signed-off-by: Brian Masney 
Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision")
Acked-by: Pavel Machek 
---
 drivers/video/backlight/lm3630a_bl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/lm3630a_bl.c 
b/drivers/video/backlight/lm3630a_bl.c
index 2030a6b77a09..ef2553f452ca 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -201,7 +201,7 @@ static int lm3630a_bank_a_update_status(struct 
backlight_device *bl)
  LM3630A_LEDA_ENABLE, LM3630A_LEDA_ENABLE);
if (ret < 0)
goto out_i2c_err;
-   return bl->props.brightness;
+   return 0;
 
 out_i2c_err:
dev_err(pchip->dev, "i2c failed to access\n");
@@ -278,7 +278,7 @@ static int lm3630a_bank_b_update_status(struct 
backlight_device *bl)
  LM3630A_LEDB_ENABLE, LM3630A_LEDB_ENABLE);
if (ret < 0)
goto out_i2c_err;
-   return bl->props.brightness;
+   return 0;
 
 out_i2c_err:
dev_err(pchip->dev, "i2c failed to access REG_CTRL\n");
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110214] radeonsi: xterm scrollback buffer disappears while Shift+PgUp and Shift+PgDn

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110214

--- Comment #81 from Diego Viola  ---
(In reply to Michel Dänzer from comment #80)
> (In reply to Diego Viola from comment #79)
> > Which might indicate this being a freetype problem?
> 
> No. If it was a FreeType problem, R600_DEBUG=nodpbb or
> MESA_EXTENSION_OVERRIDE="-GL_NV_texture_barrier" couldn't work around it.
> 
> Changing the xterm configuration just happens to avoid the problem somehow.

Makes sense, thanks.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: fix spelling mistake "resseting" -> "resetting"

2019-04-18 Thread Mukesh Ojha


On 4/18/2019 4:36 PM, Colin King wrote:

From: Colin Ian King 

There is a spelling mistake in a gvt_dbg_core debug message. Fix it.

Signed-off-by: Colin Ian King 



Reviewed-by: Mukesh Ojha 

Cheers,
-Mukesh


---
  drivers/gpu/drm/i915/gvt/vgpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index 44ce3c2b9ac1..d1e818f2b521 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -533,7 +533,7 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, 
bool dmlr,
intel_engine_mask_t resetting_eng = dmlr ? ALL_ENGINES : engine_mask;
  
  	gvt_dbg_core("--\n");

-   gvt_dbg_core("resseting vgpu%d, dmlr %d, engine_mask %08x\n",
+   gvt_dbg_core("resetting vgpu%d, dmlr %d, engine_mask %08x\n",
 vgpu->id, dmlr, engine_mask);
  
  	vgpu->resetting_eng = resetting_eng;

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/3] dt-bindings: backlight: add lm3630a bindings

2019-04-18 Thread Brian Masney
Add new backlight bindings for the TI LM3630A dual-string white LED.

Signed-off-by: Brian Masney 
---
Changes since v4:
- Drop $ref from led-sources
- Drop description from reg of i2c address
- Expand description of reg for the control bank
- Drop status from examples

Changes since v3:
- Add label. I didn't add a description for it since that'll come from
  the common properties once its converted.

Changes since v2:
- Update description of max-brightness
- Add description for reg
- Correct typo: s/tranisiton/transition
- add reg to control banks
- add additionalProperties

 .../leds/backlight/lm3630a-backlight.yaml | 129 ++
 1 file changed, 129 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml

diff --git 
a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml 
b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
new file mode 100644
index ..e754df569365
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/backlight/lm3630a-backlight.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI LM3630A High-Efficiency Dual-String White LED
+
+maintainers:
+  - Lee Jones 
+  - Daniel Thompson 
+  - Jingoo Han 
+
+description: |
+  The LM3630A is a current-mode boost converter which supplies the power and
+  controls the current in up to two strings of 10 LEDs per string.
+  https://www.ti.com/product/LM3630A
+
+properties:
+  compatible:
+const: ti,lm3630a
+
+  reg:
+maxItems: 1
+
+  ti,linear-mapping-mode:
+description: |
+  Enable linear mapping mode. If disabled, then it will use exponential
+  mapping mode in which the ramp up/down appears to have a more uniform
+  transition to the human eye.
+type: boolean
+
+required:
+  - compatible
+  - reg
+
+patternProperties:
+  "^led@[01]$":
+type: object
+description: |
+  Properties for a string of connected LEDs.
+
+properties:
+  reg:
+description: |
+  The control bank that is used to program the two current sinks. The
+  LM3630A has two control banks (A and B) and are represented as 0 or 1
+  in this property. The two current sinks can be controlled
+  independently with both banks, or bank A can be configured to control
+  both sinks with the led-sources property.
+maxItems: 1
+minimum: 0
+maximum: 1
+
+  label:
+maxItems: 1
+
+  led-sources:
+allOf:
+  - minItems: 1
+maxItems: 2
+items:
+  minimum: 0
+  maximum: 1
+
+  default-brightness:
+description: Default brightness level on boot.
+minimum: 0
+maximum: 255
+
+  max-brightness:
+description: Maximum brightness that is allowed during runtime.
+minimum: 0
+maximum: 255
+
+required:
+  - reg
+
+additionalProperties: false
+
+additionalProperties: false
+
+examples:
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+lm3630a_bl@38 {
+compatible = "ti,lm3630a";
+reg = <0x38>;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+led@0 {
+reg = <0>;
+led-sources = <0 1>;
+label = "lcd-backlight";
+default-brightness = <200>;
+max-brightness = <255>;
+};
+};
+};
+  - |
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+lm3630a_bl@38 {
+compatible = "ti,lm3630a";
+reg = <0x38>;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+led@0 {
+reg = <0>;
+default-brightness = <150>;
+ti,linear-mapping-mode;
+};
+
+led@1 {
+reg = <1>;
+default-brightness = <225>;
+ti,linear-mapping-mode;
+};
+};
+};
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 3/3] backlight: lm3630a: add firmware node support

2019-04-18 Thread Brian Masney
Add fwnode support to the lm3630a driver and optionally allow
configuring the label, default brightness level, and maximum brightness
level. The two outputs can be controlled by bank A and B independently
or bank A can control both outputs.

If the platform data was not configured, then the driver defaults to
enabling both banks. This patch changes the default value to disable
both banks before parsing the firmware node so that just a single bank
can be enabled if desired. There are no in-tree users of this driver.

Driver was tested on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney 
---
Changes since v4
- Added new function lm3630a_parse_bank()
- Renamed seen variable to seen_led_sources
- Use the bitmask returned from lm3630a_parse_led_sources() to compare
  against the seen_led_sources rather than just the control bank. This
  eliminated another if statement that was previously present that
  checked to see if control bank A should control both sinks.
- #define LM3630A_BANK_0, LM3630A_BANK_1, LM3630A_SINK_0,
  LM3630A_SINK_1, and LM3630A_NUM_SINKS and use where appropriate.
- Changed all occurances of
  'if (bank == 0) { BANK_A_WORK } else { BANK_B_WORK }' to
  'if (bank) { BANK_B_WORK } else { BANK_A_WORK }'
- Dropped unnecessary 'ret = 0' from lm3630a_parse_led_sources().
- Changed 'if (ret < 0)' to 'if (ret)' in lm3630a_parse_node().
- Dropped kerneldoc from lm3630a_parse_led_sources().

Changes since v3
- Add support for label
- Changed lm3630a_parse_node() to return -ENODEV if no nodes were found
- Remove LM3630A_LED{A,B}_DISABLE
- Add two additional newlines for code readability
- Remove extra newline

Changes since v2
- Separated out control banks and outputs via the reg and led-sources
  properties.
- Use fwnode instead of of API
- Disable both banks by default before calling lm3630a_parse_node()
- Add lots of error handling
- Check for duplicate source / bank definitions
- Remove extra ;
- Make probe() method fail if fwnode parsing fails.

 drivers/video/backlight/lm3630a_bl.c | 149 ++-
 include/linux/platform_data/lm3630a_bl.h |   4 +
 2 files changed, 148 insertions(+), 5 deletions(-)

diff --git a/drivers/video/backlight/lm3630a_bl.c 
b/drivers/video/backlight/lm3630a_bl.c
index ef2553f452ca..75d996490cf0 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -35,6 +35,14 @@
 #define REG_MAX0x50
 
 #define INT_DEBOUNCE_MSEC  10
+
+#define LM3630A_BANK_0 0
+#define LM3630A_BANK_1 1
+
+#define LM3630A_NUM_SINKS  2
+#define LM3630A_SINK_0 0
+#define LM3630A_SINK_1 1
+
 struct lm3630a_chip {
struct device *dev;
struct delayed_work work;
@@ -329,15 +337,17 @@ static const struct backlight_ops lm3630a_bank_b_ops = {
 
 static int lm3630a_backlight_register(struct lm3630a_chip *pchip)
 {
-   struct backlight_properties props;
struct lm3630a_platform_data *pdata = pchip->pdata;
+   struct backlight_properties props;
+   const char *label;
 
props.type = BACKLIGHT_RAW;
if (pdata->leda_ctrl != LM3630A_LEDA_DISABLE) {
props.brightness = pdata->leda_init_brt;
props.max_brightness = pdata->leda_max_brt;
+   label = pdata->leda_label ? pdata->leda_label : "lm3630a_leda";
pchip->bleda =
-   devm_backlight_device_register(pchip->dev, "lm3630a_leda",
+   devm_backlight_device_register(pchip->dev, label,
   pchip->dev, pchip,
   &lm3630a_bank_a_ops, &props);
if (IS_ERR(pchip->bleda))
@@ -348,8 +358,9 @@ static int lm3630a_backlight_register(struct lm3630a_chip 
*pchip)
(pdata->ledb_ctrl != LM3630A_LEDB_ON_A)) {
props.brightness = pdata->ledb_init_brt;
props.max_brightness = pdata->ledb_max_brt;
+   label = pdata->ledb_label ? pdata->ledb_label : "lm3630a_ledb";
pchip->bledb =
-   devm_backlight_device_register(pchip->dev, "lm3630a_ledb",
+   devm_backlight_device_register(pchip->dev, label,
   pchip->dev, pchip,
   &lm3630a_bank_b_ops, &props);
if (IS_ERR(pchip->bledb))
@@ -364,6 +375,123 @@ static const struct regmap_config lm3630a_regmap = {
.max_register = REG_MAX,
 };
 
+static int lm3630a_parse_led_sources(struct fwnode_handle *node,
+int default_led_sources)
+{
+   u32 sources[LM3630A_NUM_SINKS];
+   int ret, num_sources, i;
+
+   num_sources = fwnode_property_read_u32_array(node, "led-sources", NULL,
+0);
+   if (num_sources < 0)
+   return default_led_sources;
+   else if (num_so

[patch V2 00/29] stacktrace: Consolidate stack trace usage

2019-04-18 Thread Thomas Gleixner
This is an update to V1:

 https://lkml.kernel.org/r/20190410102754.387743...@linutronix.de

Struct stack_trace is a sinkhole for input and output parameters which is
largely pointless for most usage sites. In fact if embedded into other data
structures it creates indirections and extra storage overhead for no
benefit.

Looking at all usage sites makes it clear that they just require an
interface which is based on a storage array. That array is either on stack,
global or embedded into some other data structure.

Some of the stack depot usage sites are outright wrong, but fortunately the
wrongness just causes more stack being used for nothing and does not have
functional impact.

Fix this up by:

  1) Providing plain storage array based interfaces for stacktrace and
 stackdepot.

  2) Cleaning up the mess at the callsites including some related
 cleanups.

  3) Removing the struct stack_trace based interfaces

  This is not yet changing the struct stack_trace interfaces at the
  architecture level, but it removes the exposure to the usage sites.

The last two patches are extending the cleanup to the architecture level by
replacing the various save_stack_trace.* architecture interfaces with a
more unified arch_stack_walk() interface. x86 is converted, but I have
worked through all architectures already and it removes lots of duplicated
code and allows consolidation across the board. The rest of the
architecture patches are not included in this posting as I want to get
feedback on the approach itself. The diffstat of cleaning up the remaining
architectures is currently on top of the current lot is:

   47 files changed, 402 insertions(+), 1196 deletions(-)

Once this has settled, the core interfaces can be improved by adding
features, which allow to get rid of the imprecise 'skip number of entries'
approach which tries to remove the stack tracer and the callsites themself
from the trace. That's error prone due to inlining and other issues. Having
e.g. a _RET_IP_ based filter allows to do that far more reliable.

The series is based on:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/stacktrace

which contains the removal of the inconsistent and pointless ULONG_MAX
termination of stacktraces.

It's also available from git:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.core/stacktrace

   up to:  131038eb3e2f ("x86/stacktrace: Use common infrastructure")

Changes vs. V1:

   - Applied the ULONG_MAX termination cleanup in tip

   - Addressed the review comments

   - Fixed up the last users of struct stack_trace outside the stacktrace
 core and architecture code (livepatch, tracing)

   - Added the new arch_stack_walk() model and converted x86 to it

Thanks,

tglx

---
 arch/x86/Kconfig  |1 
 arch/x86/kernel/stacktrace.c  |  116 +
 drivers/gpu/drm/drm_mm.c  |   22 -
 drivers/gpu/drm/i915/i915_vma.c   |   11 
 drivers/gpu/drm/i915/intel_runtime_pm.c   |   21 -
 drivers/md/dm-bufio.c |   15 -
 drivers/md/persistent-data/dm-block-manager.c |   19 -
 fs/btrfs/ref-verify.c |   15 -
 fs/proc/base.c|   14 -
 include/linux/ftrace.h|   18 -
 include/linux/lockdep.h   |9 
 include/linux/stackdepot.h|8 
 include/linux/stacktrace.h|   80 +-
 kernel/backtracetest.c|   11 
 kernel/dma/debug.c|   13 -
 kernel/latencytop.c   |   17 -
 kernel/livepatch/transition.c |   22 -
 kernel/locking/lockdep.c  |   81 ++
 kernel/stacktrace.c   |  323 --
 kernel/trace/trace.c  |  105 +++-
 kernel/trace/trace.h  |8 
 kernel/trace/trace_events_hist.c  |   12 
 kernel/trace/trace_stack.c|   76 ++
 lib/Kconfig   |4 
 lib/fault-inject.c|   12 
 lib/stackdepot.c  |   50 ++--
 mm/kasan/common.c |   30 --
 mm/kasan/report.c |7 
 mm/kmemleak.c |   24 -
 mm/page_owner.c   |   79 ++
 mm/slub.c |   12 
 31 files changed, 664 insertions(+), 571 deletions(-)



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[patch V2 02/29] stacktrace: Provide helpers for common stack trace operations

2019-04-18 Thread Thomas Gleixner
All operations with stack traces are based on struct stack_trace. That's a
horrible construct as the struct is a kitchen sink for input and
output. Quite some usage sites embed it into their own data structures
which creates weird indirections.

There is absolutely no point in doing so. For all use cases a storage array
and the number of valid stack trace entries in the array is sufficient.

Provide helper functions which avoid the struct stack_trace indirection so
the usage sites can be cleaned up.

Signed-off-by: Thomas Gleixner 
---
 include/linux/stacktrace.h |   27 +++
 kernel/stacktrace.c|  160 -
 2 files changed, 172 insertions(+), 15 deletions(-)

--- a/include/linux/stacktrace.h
+++ b/include/linux/stacktrace.h
@@ -3,11 +3,26 @@
 #define __LINUX_STACKTRACE_H
 
 #include 
+#include 
 
 struct task_struct;
 struct pt_regs;
 
 #ifdef CONFIG_STACKTRACE
+void stack_trace_print(unsigned long *trace, unsigned int nr_entries,
+  int spaces);
+int stack_trace_snprint(char *buf, size_t size, unsigned long *entries,
+   unsigned int nr_entries, int spaces);
+unsigned int stack_trace_save(unsigned long *store, unsigned int size,
+ unsigned int skipnr);
+unsigned int stack_trace_save_tsk(struct task_struct *task,
+ unsigned long *store, unsigned int size,
+ unsigned int skipnr);
+unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
+  unsigned int size, unsigned int skipnr);
+unsigned int stack_trace_save_user(unsigned long *store, unsigned int size);
+
+/* Internal interfaces. Do not use in generic code */
 struct stack_trace {
unsigned int nr_entries, max_entries;
unsigned long *entries;
@@ -41,4 +56,16 @@ extern void save_stack_trace_user(struct
 # define save_stack_trace_tsk_reliable(tsk, trace) ({ -ENOSYS; })
 #endif /* CONFIG_STACKTRACE */
 
+#if defined(CONFIG_STACKTRACE) && defined(CONFIG_HAVE_RELIABLE_STACKTRACE)
+int stack_trace_save_tsk_reliable(struct task_struct *tsk, unsigned long 
*store,
+ unsigned int size);
+#else
+static inline int stack_trace_save_tsk_reliable(struct task_struct *tsk,
+   unsigned long *store,
+   unsigned int size)
+{
+   return -ENOSYS;
+}
+#endif
+
 #endif /* __LINUX_STACKTRACE_H */
--- a/kernel/stacktrace.c
+++ b/kernel/stacktrace.c
@@ -11,35 +11,52 @@
 #include 
 #include 
 
-void print_stack_trace(struct stack_trace *trace, int spaces)
+/**
+ * stack_trace_print - Print the entries in the stack trace
+ * @entries:   Pointer to storage array
+ * @nr_entries:Number of entries in the storage array
+ * @spaces:Number of leading spaces to print
+ */
+void stack_trace_print(unsigned long *entries, unsigned int nr_entries,
+  int spaces)
 {
-   int i;
+   unsigned int i;
 
-   if (WARN_ON(!trace->entries))
+   if (WARN_ON(!entries))
return;
 
-   for (i = 0; i < trace->nr_entries; i++)
-   printk("%*c%pS\n", 1 + spaces, ' ', (void *)trace->entries[i]);
+   for (i = 0; i < nr_entries; i++)
+   printk("%*c%pS\n", 1 + spaces, ' ', (void *)entries[i]);
+}
+EXPORT_SYMBOL_GPL(stack_trace_print);
+
+void print_stack_trace(struct stack_trace *trace, int spaces)
+{
+   stack_trace_print(trace->entries, trace->nr_entries, spaces);
 }
 EXPORT_SYMBOL_GPL(print_stack_trace);
 
-int snprint_stack_trace(char *buf, size_t size,
-   struct stack_trace *trace, int spaces)
+/**
+ * stack_trace_snprint - Print the entries in the stack trace into a buffer
+ * @buf:   Pointer to the print buffer
+ * @size:  Size of the print buffer
+ * @entries:   Pointer to storage array
+ * @nr_entries:Number of entries in the storage array
+ * @spaces:Number of leading spaces to print
+ */
+int stack_trace_snprint(char *buf, size_t size, unsigned long *entries,
+   unsigned int nr_entries, int spaces)
 {
-   int i;
-   int generated;
-   int total = 0;
+   unsigned int generated, i, total = 0;
 
-   if (WARN_ON(!trace->entries))
+   if (WARN_ON(!entries))
return 0;
 
-   for (i = 0; i < trace->nr_entries; i++) {
+   for (i = 0; i < nr_entries; i++) {
generated = snprintf(buf, size, "%*c%pS\n", 1 + spaces, ' ',
-(void *)trace->entries[i]);
+(void *)entries[i]);
 
total += generated;
-
-   /* Assume that generated isn't a negative number */
if (generated >= size) {
buf += size;
size = 0;
@@ -51,6 +68,14 @@ int snprint_stack_trace(char *buf, size_
 

[patch V2 16/29] drm: Simplify stacktrace handling

2019-04-18 Thread Thomas Gleixner
Replace the indirection through struct stack_trace by using the storage
array based interfaces.

The original code in all printing functions is really wrong. It allocates a
storage array on stack which is unused because depot_fetch_stack() does not
store anything in it. It overwrites the entries pointer in the stack_trace
struct so it points to the depot storage.

Signed-off-by: Thomas Gleixner 
Cc: intel-...@lists.freedesktop.org
Cc: Joonas Lahtinen 
Cc: Maarten Lankhorst 
Cc: dri-devel@lists.freedesktop.org
Cc: David Airlie 
Cc: Jani Nikula 
Cc: Daniel Vetter 
Cc: Rodrigo Vivi 
---
 drivers/gpu/drm/drm_mm.c|   22 +++---
 drivers/gpu/drm/i915/i915_vma.c |   11 ---
 drivers/gpu/drm/i915/intel_runtime_pm.c |   21 +++--
 3 files changed, 18 insertions(+), 36 deletions(-)

--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -106,22 +106,19 @@
 static noinline void save_stack(struct drm_mm_node *node)
 {
unsigned long entries[STACKDEPTH];
-   struct stack_trace trace = {
-   .entries = entries,
-   .max_entries = STACKDEPTH,
-   .skip = 1
-   };
+   unsigned int n;
 
-   save_stack_trace(&trace);
+   n = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
 
/* May be called under spinlock, so avoid sleeping */
-   node->stack = depot_save_stack(&trace, GFP_NOWAIT);
+   node->stack = stack_depot_save(entries, n, GFP_NOWAIT);
 }
 
 static void show_leaks(struct drm_mm *mm)
 {
struct drm_mm_node *node;
-   unsigned long entries[STACKDEPTH];
+   unsigned long *entries;
+   unsigned int nr_entries;
char *buf;
 
buf = kmalloc(BUFSZ, GFP_KERNEL);
@@ -129,19 +126,14 @@ static void show_leaks(struct drm_mm *mm
return;
 
list_for_each_entry(node, drm_mm_nodes(mm), node_list) {
-   struct stack_trace trace = {
-   .entries = entries,
-   .max_entries = STACKDEPTH
-   };
-
if (!node->stack) {
DRM_ERROR("node [%08llx + %08llx]: unknown owner\n",
  node->start, node->size);
continue;
}
 
-   depot_fetch_stack(node->stack, &trace);
-   snprint_stack_trace(buf, BUFSZ, &trace, 0);
+   nr_entries = stack_depot_fetch(node->stack, &entries);
+   stack_trace_snprint(buf, BUFSZ, entries, nr_entries, 0);
DRM_ERROR("node [%08llx + %08llx]: inserted at\n%s",
  node->start, node->size, buf);
}
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -36,11 +36,8 @@
 
 static void vma_print_allocator(struct i915_vma *vma, const char *reason)
 {
-   unsigned long entries[12];
-   struct stack_trace trace = {
-   .entries = entries,
-   .max_entries = ARRAY_SIZE(entries),
-   };
+   unsigned long *entries;
+   unsigned int nr_entries;
char buf[512];
 
if (!vma->node.stack) {
@@ -49,8 +46,8 @@ static void vma_print_allocator(struct i
return;
}
 
-   depot_fetch_stack(vma->node.stack, &trace);
-   snprint_stack_trace(buf, sizeof(buf), &trace, 0);
+   nr_entries = stack_depot_fetch(vma->node.stack, &entries);
+   stack_trace_snprint(buf, sizeof(buf), entries, nr_entries, 0);
DRM_DEBUG_DRIVER("vma.node [%08llx + %08llx] %s: inserted at %s\n",
 vma->node.start, vma->node.size, reason, buf);
 }
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -60,27 +60,20 @@
 static noinline depot_stack_handle_t __save_depot_stack(void)
 {
unsigned long entries[STACKDEPTH];
-   struct stack_trace trace = {
-   .entries = entries,
-   .max_entries = ARRAY_SIZE(entries),
-   .skip = 1,
-   };
+   unsigned int n;
 
-   save_stack_trace(&trace);
-   return depot_save_stack(&trace, GFP_NOWAIT | __GFP_NOWARN);
+   n = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
+   return stack_depot_save(entries, n, GFP_NOWAIT | __GFP_NOWARN);
 }
 
 static void __print_depot_stack(depot_stack_handle_t stack,
char *buf, int sz, int indent)
 {
-   unsigned long entries[STACKDEPTH];
-   struct stack_trace trace = {
-   .entries = entries,
-   .max_entries = ARRAY_SIZE(entries),
-   };
+   unsigned long *entries;
+   unsigned int nr_entries;
 
-   depot_fetch_stack(stack, &trace);
-   snprint_stack_trace(buf, sz, &trace, indent);
+   nr_entries = stack_depot_fetch(stack, &entries);
+   stack_trace_snprint(buf, sz, entries, nr_entries, indent);
 }
 
 static void init_intel_runtime_pm_wakeref(struct drm_i915_private *i915)



[Bug 110417] regression drm-next-5.2-wip: breaks vsync of new Compton xrender backend

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110417

Michel Dänzer  changed:

   What|Removed |Added

  Component|DRM/AMDgpu  |Driver/AMDgpu
Version|DRI git |git
 QA Contact||xorg-t...@lists.x.org
Product|DRI |xorg
   Assignee|dri-devel@lists.freedesktop |xorg-driver-...@lists.x.org
   |.org|

--- Comment #2 from Michel Dänzer  ---
https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/merge_requests/33
fixes this for me.

-- 
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: [linux-sunxi] [PATCH 3/3] drm/sun4i: Fix component unbinding and component master deletion

2019-04-18 Thread Paul Kocialkowski
Hi,

Le jeudi 18 avril 2019 à 08:03 -0700, Chen-Yu Tsai a écrit :
> On Thu, Apr 18, 2019 at 6:27 AM Paul Kocialkowski
>  wrote:
> > For our component-backed driver to be properly removed, we need to
> > delete the component master in sun4i_drv_remove and make sure to call
> > component_unbind_all in the master's unbind so that all components are
> > unbound when the master is.
> > 
> > Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  drivers/gpu/drm/sun4i/sun4i_drv.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
> > b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > index af07291544a4..0ea365e54de1 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> > @@ -137,6 +137,8 @@ static void sun4i_drv_unbind(struct device *dev)
> > drm_mode_config_cleanup(drm);
> > of_reserved_mem_device_release(dev);
> > drm_dev_put(drm);
> > +
> > +   component_unbind_all(dev, NULL);
> 
> Shouldn't this be before drm_dev_put? Everything being in reverse order
> of the complement calls in the bind function and all. The component
> drivers might still be using the drm dev before they are unbound.

Mhh, yes that's a valid concern, thanks for pointing it out. I'll send
out a fix for it tomorrow.

Cheers,

Paul

> ChenYu
> 
> >  }
> > 
> >  static const struct component_master_ops sun4i_drv_master_ops = {
> > @@ -385,6 +387,8 @@ static int sun4i_drv_probe(struct platform_device *pdev)
> > 
> >  static int sun4i_drv_remove(struct platform_device *pdev)
> >  {
> > +   component_master_del(&pdev->dev, &sun4i_drv_master_ops);
> > +
> > return 0;
> >  }
> > 
> > --
> > 2.21.0
> > 
> > --
> > You received this message because you are subscribed to the Google Groups 
> > "linux-sunxi" group.
> > To unsubscribe from this group and stop receiving emails from it, send an 
> > email to linux-sunxi+unsubscr...@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 6/6] drm/vc4: hdmi: Set default state margin at reset

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> Now that the TV margins are properly parsed and filled into
> drm_cmdline_mode, we just need to initialise the first state at reset to
> get those values and start using them.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/vc4/vc4_hdmi.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 99fc8569e0f5..d86f154138f5 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -255,11 +255,25 @@ static int vc4_hdmi_connector_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> +static void vc4_hdmi_connector_reset(struct drm_connector *connector)
> +{
> + struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
> + struct drm_connector_state *state;
> +
> + drm_atomic_helper_connector_reset(connector);

Initially I wondered why not do this in the helper, but ofc that would
apply the values to all the connectors. I know too little about this
subject to argue for having it in the helper, and it can be changed
later if deemed convinient, so:

Acked-by: Noralf Trønnes 

> +
> + state = connector->state;
> + state->tv.margins.left = cmdline->tv_margins.left;
> + state->tv.margins.right = cmdline->tv_margins.right;
> + state->tv.margins.top = cmdline->tv_margins.top;
> + state->tv.margins.bottom = cmdline->tv_margins.bottom;
> +}
> +
>  static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
>   .detect = vc4_hdmi_connector_detect,
>   .fill_modes = drm_helper_probe_single_connector_modes,
>   .destroy = vc4_hdmi_connector_destroy,
> - .reset = drm_atomic_helper_connector_reset,
> + .reset = vc4_hdmi_connector_reset,
>   .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>  };
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/nouveau/fb/ramgk104: fix spelling mistake "sucessfully" -> "successfully"

2019-04-18 Thread Colin King
From: Colin Ian King 

There is a spelling mistake in a nvkm_debug message. Fix it.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
index 8bcb7e79a0cb..456aed1f2a02 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
@@ -1070,7 +1070,7 @@ gk104_ram_calc_xits(struct gk104_ram *ram, struct 
nvkm_ram_data *next)
nvkm_error(subdev, "unable to calc plls\n");
return -EINVAL;
}
-   nvkm_debug(subdev, "sucessfully calced PLLs for clock %i kHz"
+   nvkm_debug(subdev, "successfully calced PLLs for clock %i kHz"
" (refclock: %i kHz)\n", next->freq, ret);
} else {
/* calculate refpll coefficients */
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 5/6] drm/selftests: Add command line parser selftests

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> The command line parser is pretty tough to get right and very error prone,
> so let's add a selftest to try to catch any regression.
> 
> Signed-off-by: Maxime Ripard 
> ---

Yay, unit tests \o/

Reviewed-by: Noralf Trønnes 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110443] vaapi/vpp: wrong output for non 64-bytes align width (ex: 1200)

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110443

--- Comment #4 from Julien Isorce  ---
Something like this:

--- a/src/gallium/include/pipe/p_screen.h
+++ b/src/gallium/include/pipe/p_screen.h
@@ -262,6 +262,15 @@ struct pipe_screen {
  struct winsys_handle *handle,
  unsigned usage);

+   boolean (*resource_get_stride)(struct pipe_screen *screen,
+  struct pipe_context *context,
+  struct pipe_resource *tex,
+  unsigned *stride);
+   boolean (*resource_get_offset)(struct pipe_screen *screen,
+  struct pipe_context *context,
+  struct pipe_resource *tex,
+  unsigned *offset);
+

? Thx

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/6] drm/modes: Parse overscan properties

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> Properly configuring the overscan properties might be needed for the
> initial setup of the framebuffer for display that still have overscan.
> Let's allow for more properties on the kernel command line to setup each
> margin.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_modes.c | 44 ++-
>  include/drm/drm_connector.h | 14 -
>  2 files changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac8d70b92b62..d93c44a97ce9 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str, 
> size_t len,
>   } else if (!strncmp(option, "reflect_y", delim - option)) {
>   rotation |= DRM_MODE_REFLECT_Y;
>   sep = delim;
> + } else if (!strncmp(option, "margin_right", delim - option)) {
> + const char *value = delim + 1;
> + unsigned int margin;
> +
> + margin = simple_strtol(value, &sep, 10);
> +
> + /* Make sure we have parsed something */
> + if (sep == value)
> + return -EINVAL;
> +
> + mode->tv_margins.right = margin;
> + } else if (!strncmp(option, "margin_left", delim - option)) {
> + const char *value = delim + 1;
> + unsigned int margin;
> +
> + margin = simple_strtol(value, &sep, 10);
> +
> + /* Make sure we have parsed something */
> + if (sep == value)
> + return -EINVAL;
> +
> + mode->tv_margins.left = margin;
> + } else if (!strncmp(option, "margin_top", delim - option)) {
> + const char *value = delim + 1;
> + unsigned int margin;
> +
> + margin = simple_strtol(value, &sep, 10);
> +
> + /* Make sure we have parsed something */
> + if (sep == value)
> + return -EINVAL;
> +
> + mode->tv_margins.top = margin;
> + } else if (!strncmp(option, "margin_bottom", delim - option)) {
> + const char *value = delim + 1;
> + unsigned int margin;
> +
> + margin = simple_strtol(value, &sep, 10);
> +
> + /* Make sure we have parsed something */
> + if (sep == value)
> + return -EINVAL;
> +
> + mode->tv_margins.bottom = margin;
>   } else {
>   return -EINVAL;
>   }
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 6f57c1a3afff..89bc6ac38043 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -917,6 +917,20 @@ struct drm_cmdline_mode {
>* DRM_MODE_ROTATE_180 are supported at the moment.
>*/
>   unsigned int rotation;
> +
> + /**
> +  * @tv_margins: TV margins (in pixels)
> +  * @tv_margins.left: left margin
> +  * @tv_margins.right: right margin
> +  * @tv_margins.top: top margin
> +  * @tv_margins.bottom: bottom margin
> +  */

I haven't seen kernel docs like this before so I assume you have tested
with make htmldocs.

This one also needs mention in Documentation/fb/modedb.txt. With that:

Reviewed-by: Noralf Trønnes 

> + struct {
> + unsigned int left;
> + unsigned int right;
> + unsigned int top;
> + unsigned int bottom;
> + } tv_margins;
>  };
>  
>  /**
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> Rotations and reflections setup are needed in some scenarios to initialise
> properly the initial framebuffer. Some drivers already had a bunch of
> quirks to deal with this, such as either a private kernel command line
> parameter (omapdss) or on the device tree (various panels).
> 
> In order to accomodate this, let's create a video mode parameter to deal
> with the rotation and reflexion.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_client_modeset.c |  10 +++-
>  drivers/gpu/drm/drm_modes.c  | 110 ++--
>  include/drm/drm_connector.h  |   9 ++-
>  3 files changed, 109 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> b/drivers/gpu/drm/drm_client_modeset.c
> index f2869c82510c..15145d2c86d5 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe);
>  bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int 
> *rotation)
>  {
>   struct drm_connector *connector = modeset->connectors[0];
> + struct drm_cmdline_mode *cmdline;
>   struct drm_plane *plane = modeset->crtc->primary;
>   u64 valid_mask = 0;
>   unsigned int i;
> @@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set 
> *modeset, unsigned int *rotat
>   *rotation = DRM_MODE_ROTATE_0;
>   }
>  
> + /**
> +  * We want the rotation on the command line to overwrite
> +  * whatever comes from the panel.
> +  */
> + cmdline = &connector->cmdline_mode;
> + if (cmdline->specified &&
> + cmdline->rotation != DRM_MODE_ROTATE_0)

I believe you need to drop that second check, otherwise rotate=0 will
not overwrite panel rotation.

> + *rotation = cmdline->rotation;
> +
>   /*
>* TODO: support 90 / 270 degree hardware rotation,
>* depending on the hardware this may require the framebuffer
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 9613c1a28487..ac8d70b92b62 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char 
> *str, unsigned int length,
>   return 0;
>  }
>  
> +static int drm_mode_parse_cmdline_options(char *str, size_t len,
> +   struct drm_connector *connector,
> +   struct drm_cmdline_mode *mode)
> +{
> + unsigned int rotation = 0;
> + char *sep = str;
> +
> + while ((sep = strchr(sep, ','))) {
> + char *delim, *option;
> +
> + option = sep + 1;
> + delim = strchr(option, '=');
> + if (!delim) {
> + delim = strchr(option, ',');
> +
> + if (!delim)
> + delim = str + len;
> + }
> +
> + if (!strncmp(option, "rotate", delim - option)) {
> + const char *value = delim + 1;
> + unsigned int deg;
> +
> + deg = simple_strtol(value, &sep, 10);
> +
> + /* Make sure we have parsed something */
> + if (sep == value)
> + return -EINVAL;
> +
> + switch (deg) {
> + case 0:
> + rotation |= DRM_MODE_ROTATE_0;
> + break;
> +
> + case 90:
> + rotation |= DRM_MODE_ROTATE_90;
> + break;
> +
> + case 180:
> + rotation |= DRM_MODE_ROTATE_180;
> + break;
> +
> + case 270:
> + rotation |= DRM_MODE_ROTATE_270;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> + } else if (!strncmp(option, "reflect_x", delim - option)) {
> + rotation |= DRM_MODE_REFLECT_X;
> + sep = delim;
> + } else if (!strncmp(option, "reflect_y", delim - option)) {

I think you should drop reflect_x and _y for now since they're not
supported. People like me that only reads code and not documentation
(ahem..) will get the impression that this should work.

Documentation/fb/modedb.txt needs to be updated with this new video= option.

Noralf.

> + rotation |= DRM_MODE_REFLECT_Y;
> + sep = delim;
> + } else {
> + return -EINVAL;
> + }
> + }
> +
> + mode->rotation = rotation;
> +
> + return 0;
> +}
> +
>  /**
>   * drm_mode_parse_command_line_for_connector - parse command line modeline 
> for connector
>   

Re: [Intel-gfx] [PATCH v2] drm: Fire off KMS hotplug events if probe detect says the connector is connected

2019-04-18 Thread Jani Nikula
On Thu, 18 Apr 2019, Gwan-gyeong Mun  wrote:
> The hotplug detection routine of drm_helper_hpd_irq_event() can detect
> changing of status of connector, but it can not detect changing of
> properties of the connector.
> e.g. changing of edid while suspend/resume, changing of dp lanes in dp aux.
>
> Following scenario explains one of them; A detection of changing of edid.
>
>  1) plug display device to a connector
>  2) system suspend
>  3) unplug 1)'s display device and plug the other display device to a
> connector
>  4) system resume
>
> To solve explained cases, It fires off KMS hotplug events if
> drm_helper_probe_detect() says the connector is connected.
>
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
>
> v2: Remove suggested-by line (danvet)
> Signed-off-by: Gwan-gyeong Mun 
> Link: https://lists.freedesktop.org/archives/dri-devel/2019-April/214572.html

Link: is something added by our tooling (dim), not to be added
yourself. It points at the patch at patchwork using the message-id of
the patch email.

If you want to refer to some other work, please use References: tag.

BR,
Jani.


> ---
>  drivers/gpu/drm/drm_probe_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index 6fd08e04b323..081a849104f2 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -780,7 +780,8 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
> connector->name,
> drm_get_connector_status_name(old_status),
> drm_get_connector_status_name(connector->status));
> - if (old_status != connector->status)
> + if (old_status != connector->status ||
> + connector->status == connector_status_connected)
>   changed = true;
>   }
>   drm_connector_list_iter_end(&conn_iter);

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/6] drm/modes: Support modes names on the command line

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> From: Maxime Ripard 
> 
> The drm subsystem also uses the video= kernel parameter, and in the
> documentation refers to the fbdev documentation for that parameter.
> 
> However, that documentation also says that instead of giving the mode using
> its resolution we can also give a name. However, DRM doesn't handle that
> case at the moment. Even though in most case it shouldn't make any
> difference, it might be useful for analog modes, where different standards
> might have the same resolution, but still have a few different parameters
> that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_client_modeset.c |  4 ++-
>  drivers/gpu/drm/drm_connector.c  |  3 +-
>  drivers/gpu/drm/drm_modes.c  | 52 -
>  include/drm/drm_connector.h  |  1 +-
>  4 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c 
> b/drivers/gpu/drm/drm_client_modeset.c
> index 2a428ac00930..f2869c82510c 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -149,6 +149,10 @@ drm_connector_pick_cmdline_mode(struct drm_connector 
> *connector)
>   prefer_non_interlace = !cmdline_mode->interlace;
>  again:
>   list_for_each_entry(mode, &connector->modes, head) {
> + /* Check (optional) mode name first */
> + if (!strcmp(mode->name, cmdline_mode->name))
> + return mode;
> +
>   /* check width/height */
>   if (mode->hdisplay != cmdline_mode->xres ||
>   mode->vdisplay != cmdline_mode->yres)
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..e33814f5940e 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct 
> drm_connector *connector)
>   connector->force = mode->force;
>   }
>  
> - DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
> + DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
> connector->name,
> +   mode->name ? mode->name : "",
> mode->xres, mode->yres,
> mode->refresh_specified ? mode->refresh : 60,
> mode->rb ? " reduced blanking" : "",
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 3f89198f0891..9613c1a28487 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const 
> char *mode_option,
>  struct drm_cmdline_mode *mode)
>  {
>   const char *name;
> - bool parse_extras = false;
> + bool named_mode = false, parse_extras = false;

You don't need to assign a value here, or if you do, you can drop the
else further down.

Reviewed-by: Noralf Trønnes 

>   unsigned int bpp_off = 0, refresh_off = 0;
>   unsigned int mode_end = 0;
>   char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
> @@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const 
> char *mode_option,
>  
>   name = mode_option;
>  
> + /*
> +  * If the first character is not a digit, then it means that
> +  * we have a named mode.
> +  */
>   if (!isdigit(name[0]))
> - return false;
> + named_mode = true;
> + else
> + named_mode = false;
>  
>   /* Try to locate the bpp and refresh specifiers, if any */
>   bpp_ptr = strchr(name, '-');
> @@ -1588,6 +1594,9 @@ bool drm_mode_parse_command_line_for_connector(const 
> char *mode_option,
>  
>   refresh_ptr = strchr(name, '@');
>   if (refresh_ptr) {
> + if (named_mode)
> + return false;
> +
>   refresh_off = refresh_ptr - name;
>   mode->refresh_specified = true;
>   }
> @@ -1604,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const 
> char *mode_option,
>   parse_extras = true;
>   }
>  
> - ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
> -   parse_extras,
> -   connector,
> -   mode);
> - if (ret)
> - return false;
> + if (named_mode) {
> + strncpy(mode->name, name, mode_end);
> + } else {
> + ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
> +   parse_extras,
> +   connector,
> +   mode);
> + if (ret)
> + return

Re: [PATCH v3 1/6] drm/modes: Rewrite the command line parser

2019-04-18 Thread Noralf Trønnes


Den 18.04.2019 14.41, skrev Maxime Ripard:
> From: Maxime Ripard 
> 
> Rewrite the command line parser in order to get away from the state machine
> parsing the video mode lines.
> 
> Hopefully, this will allow to extend it more easily to support named modes
> and / or properties set directly on the command line.
> 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_modes.c | 305 +++--
>  1 file changed, 190 insertions(+), 115 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 56f92a0bba62..3f89198f0891 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -30,6 +30,7 @@
>   * authorization from the copyright holder(s) and author(s).
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1405,6 +1406,131 @@ void drm_connector_list_update(struct drm_connector 
> *connector)
>  }
>  EXPORT_SYMBOL(drm_connector_list_update);
>  
> +static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
> +   struct drm_cmdline_mode *mode)
> +{
> + if (str[0] != '-')
> + return -EINVAL;
> +
> + mode->bpp = simple_strtol(str + 1, end_ptr, 10);

What happens if this is not a number? I didn't find a test for that in
your unit tests. The same goes for _refresh().

> + mode->bpp_specified = true;
> +
> + return 0;
> +}
> +
> +static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr,
> +   struct drm_cmdline_mode *mode)
> +{
> + if (str[0] != '@')
> + return -EINVAL;
> +
> + mode->refresh = simple_strtol(str + 1, end_ptr, 10);
> + mode->refresh_specified = true;
> +
> + return 0;
> +}
> +
> +static int drm_mode_parse_cmdline_extra(const char *str, int length,
> + struct drm_connector *connector,
> + struct drm_cmdline_mode *mode)
> +{
> + int i;
> +
> + for (i = 0; i < length; i++) {
> + switch (str[i]) {
> + case 'i':
> + mode->interlace = true;
> + break;
> + case 'm':
> + mode->margins = true;
> + break;
> + case 'D':
> + if (mode->force != DRM_FORCE_UNSPECIFIED)
> + return -EINVAL;
> +
> + if ((connector->connector_type != 
> DRM_MODE_CONNECTOR_DVII) &&
> + (connector->connector_type != 
> DRM_MODE_CONNECTOR_HDMIB))
> + mode->force = DRM_FORCE_ON;
> + else
> + mode->force = DRM_FORCE_ON_DIGITAL;
> + break;
> + case 'd':
> + if (mode->force != DRM_FORCE_UNSPECIFIED)
> + return -EINVAL;
> +
> + mode->force = DRM_FORCE_OFF;
> + break;
> + case 'e':
> + if (mode->force != DRM_FORCE_UNSPECIFIED)
> + return -EINVAL;
> +
> + mode->force = DRM_FORCE_ON;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int 
> length,
> +bool extras,
> +struct drm_connector *connector,
> +struct drm_cmdline_mode *mode)
> +{
> + bool rb = false, cvt = false;
> + int xres = 0, yres = 0;
> + int remaining, i;
> + char *end_ptr;
> +
> + xres = simple_strtol(str, &end_ptr, 10);
> +
> + if (end_ptr[0] != 'x')

'x600' looks to be a valid resolution here, so I think:
if (str == end_ptr || end_ptr[0] != 'x')

> + return -EINVAL;
> + end_ptr++;
> +
> + yres = simple_strtol(end_ptr, &end_ptr, 10);
> +
> + remaining = length - (end_ptr - str);
> + if (remaining < 0)
> + return -EINVAL;

Maybe some unit tests: '1024xy' and '1024x', to verify that this does
indeed require a number for yres.

Noralf.

> +
> + for (i = 0; i < remaining; i++) {
> + switch (end_ptr[i]) {
> + case 'M':
> + cvt = true;
> + break;
> + case 'R':
> + rb = true;
> + break;
> + default:
> + /*
> +  * Try to pass that to our extras parsing
> +  * function to handle the case where the
> +  * extras are directly after the resolution
> +  */
> + if (extras) {
> + int ret = drm_mode_parse_cmdline_extra(end_ptr 
> 

Re: [patch V2 21/29] tracing: Use percpu stack trace buffer more intelligently

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 17:43:59 +0200 (CEST)
Thomas Gleixner  wrote:

> On Thu, 18 Apr 2019, Steven Rostedt wrote:
> > On Thu, 18 Apr 2019 10:41:40 +0200
> > Thomas Gleixner  wrote:  
> > > -static DEFINE_PER_CPU(struct ftrace_stack, ftrace_stack);
> > > +/* This allows 8 level nesting which is plenty */  
> > 
> > Can we make this 4 level nesting and increase the size? (I can see us
> > going more than 64 deep, kernel developers never cease to amaze me ;-)
> > That's all we need:
> > 
> >  Context: Normal, softirq, irq, NMI
> > 
> > Is there any other way to nest?  
> 
> Not that I know, but you are the tracer dude :)
>

There's other places I only test 4 deep, so it should be fine to limit
it to 4 then.

Thanks!

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110371] HP Dreamcolor display *Error* No EDID read

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110371

--- Comment #5 from Nicholas Kazlauskas  ---
(In reply to Michel Dänzer from comment #4)
> (In reply to Nicholas Kazlauskas from comment #3)
> > It's hitting the "." as part of the video mode and erroring out because DRM
> > doesn't consider it a valid character.
> 
> Or maybe there's two separate issues? Failure to parse the mode name
> shouldn't affect whether or not DC picks up EDID, should it?

I suppose that doesn't actually influence whether the EDID has been read or is
valid. So that's a different bug.

The "No EDID read." error comes from the result from drm_get_edid(...) being
NULL, however.

While we're responsible for the actual transfers to and from the receiver the
actual logic is shared there between drivers.

(In reply to Babblebones from comment #2)
> Created attachment 143957 [details]
> Dmesg output from an affected kernel
> 
> Here is the issue, you can see
> [1.335096] [drm:dc_link_detect [amdgpu]] *ERROR* No EDID read.
> 
> Which is about where the display turns into fruit salad.
> Please let me know if I can submit any more debug info that would assist! I
> am very much in need of running a newer kernel.

Can you post a dmesg log with drm.debug=4 as part of your kernel boot
parameters?

-- 
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 101473] (SI) latest stable & cant boot

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=101473

qmaster...@gmail.com changed:

   What|Removed |Added

 Resolution|FIXED   |---
 Status|RESOLVED|REOPENED

--- Comment #7 from qmaster...@gmail.com ---
Sorry, but this message
"Invalid PCI ROM header structure: expecting 0xaa55, got 0x"
is not always harmless, since it usually leads to "Fatal error during GPU init"
:

[5.732623] [drm:radeon_get_bios [radeon]] *ERROR* ACPI VFCT image header
truncated
[5.732646] radeon :01:00.0: Invalid PCI ROM header signature: expecting
0xaa55, got 0x
[5.732829] radeon :01:00.0: Invalid PCI ROM header signature: expecting
0xaa55, got 0x
[5.732866] [drm:radeon_get_bios [radeon]] *ERROR* Unable to locate a BIOS
ROM
[5.732869] radeon :01:00.0: Fatal error during GPU init
[5.732872] [drm] radeon: finishing device.

-- 
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: [git pull] drm fixes for 5.1-rc6

2019-04-18 Thread pr-tracker-bot
The pull request you sent on Thu, 18 Apr 2019 13:40:53 +1000:

> git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2019-04-18

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/95ea55291e35107f6cc1838e499e57d236a45d44

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[GIT PULL] drm/tegra: Changes for v5.2-rc1

2019-04-18 Thread Thierry Reding
Hi Dave,

The following changes since commit 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b:

  Linux 5.1-rc1 (2019-03-17 14:22:26 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-5.2-rc1

for you to fetch changes up to 61b51fb51c01a519a249d28ec55c6513a13be5a3:

  drm/tegra: gem: Fix CPU-cache maintenance for BO's allocated using 
get_pages() (2019-04-18 11:48:09 +0200)

Note that this pulls in a stable branch from Philipp for the core reset
framework changes that add the require/release protocol API.

Thanks,
Thierry


drm/tegra: Changes for v5.2-rc1

This contains a fix for the usage of shared resets that previously
generated a WARN on boot. In addition, there's a fix for CPU cache
maintenance of GEM buffers allocated using get_pages().


Dmitry Osipenko (1):
  drm/tegra: gem: Fix CPU-cache maintenance for BO's allocated using 
get_pages()

Philipp Zabel (1):
  reset: add acquired/released state for exclusive reset controls

Thierry Reding (4):
  reset: Add acquired flag to of_reset_control_array_get()
  reset: Add acquire/release support for arrays
  Merge branch 'reset/acquire' of git://git.pengutronix.de/git/pza/linux 
into drm/tegra/for-next
  drm/tegra: sor: Implement acquire/release for reset

 drivers/gpu/drm/tegra/gem.c   |   4 +-
 drivers/gpu/drm/tegra/sor.c   |  21 -
 drivers/reset/core.c  | 180 +++---
 drivers/usb/dwc3/dwc3-of-simple.c |   3 +-
 include/linux/reset.h | 113 ++--
 5 files changed, 277 insertions(+), 44 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110371] HP Dreamcolor display *Error* No EDID read

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110371

--- Comment #4 from Michel Dänzer  ---
(In reply to Nicholas Kazlauskas from comment #3)
> It's hitting the "." as part of the video mode and erroring out because DRM
> doesn't consider it a valid character.

Or maybe there's two separate issues? Failure to parse the mode name shouldn't
affect whether or not DC picks up EDID, should it?

-- 
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: [linux-sunxi] [PATCH 3/3] drm/sun4i: Fix component unbinding and component master deletion

2019-04-18 Thread Chen-Yu Tsai
On Thu, Apr 18, 2019 at 6:27 AM Paul Kocialkowski
 wrote:
>
> For our component-backed driver to be properly removed, we need to
> delete the component master in sun4i_drv_remove and make sure to call
> component_unbind_all in the master's unbind so that all components are
> unbound when the master is.
>
> Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/sun4i/sun4i_drv.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
> b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index af07291544a4..0ea365e54de1 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -137,6 +137,8 @@ static void sun4i_drv_unbind(struct device *dev)
> drm_mode_config_cleanup(drm);
> of_reserved_mem_device_release(dev);
> drm_dev_put(drm);
> +
> +   component_unbind_all(dev, NULL);

Shouldn't this be before drm_dev_put? Everything being in reverse order
of the complement calls in the bind function and all. The component
drivers might still be using the drm dev before they are unbound.

ChenYu

>  }
>
>  static const struct component_master_ops sun4i_drv_master_ops = {
> @@ -385,6 +387,8 @@ static int sun4i_drv_probe(struct platform_device *pdev)
>
>  static int sun4i_drv_remove(struct platform_device *pdev)
>  {
> +   component_master_del(&pdev->dev, &sun4i_drv_master_ops);
> +
> return 0;
>  }
>
> --
> 2.21.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 6/6] drm/amdgpu: Avoid HW reset if guilty job already signaled.

2019-04-18 Thread Andrey Grodzovsky
Also reject TDRs if another one already running.

v2:
Stop all schedulers across device and entire XGMI hive before
force signaling HW fences.
Avoid passing job_signaled to helper fnctions to keep all the decision
making about skipping HW reset in one place.

v3:
Fix SW sched. hang after non HW reset. sched.hw_rq_count has to be balanced
against it's decrement in drm_sched_stop in non HW reset case.
v4: rebase
v5: Revert v3 as we do it now in sceduler code.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 143 +++--
 1 file changed, 95 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a0e165c..85f8792 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3334,8 +3334,6 @@ static int amdgpu_device_pre_asic_reset(struct 
amdgpu_device *adev,
if (!ring || !ring->sched.thread)
continue;
 
-   drm_sched_stop(&ring->sched, &job->base);
-
/* after all hw jobs are reset, hw fence is meaningless, so 
force_completion */
amdgpu_fence_driver_force_completion(ring);
}
@@ -3343,6 +3341,7 @@ static int amdgpu_device_pre_asic_reset(struct 
amdgpu_device *adev,
if(job)
drm_sched_increase_karma(&job->base);
 
+   /* Don't suspend on bare metal if we are not going to HW reset the ASIC 
*/
if (!amdgpu_sriov_vf(adev)) {
 
if (!need_full_reset)
@@ -3480,37 +3479,21 @@ static int amdgpu_do_asic_reset(struct amdgpu_hive_info 
*hive,
return r;
 }
 
-static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev)
+static bool amdgpu_device_lock_adev(struct amdgpu_device *adev, bool trylock)
 {
-   int i;
-
-   for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
-   struct amdgpu_ring *ring = adev->rings[i];
-
-   if (!ring || !ring->sched.thread)
-   continue;
-
-   if (!adev->asic_reset_res)
-   drm_sched_resubmit_jobs(&ring->sched);
+   if (trylock) {
+   if (!mutex_trylock(&adev->lock_reset))
+   return false;
+   } else
+   mutex_lock(&adev->lock_reset);
 
-   drm_sched_start(&ring->sched, !adev->asic_reset_res);
-   }
-
-   if (!amdgpu_device_has_dc_support(adev)) {
-   drm_helper_resume_force_mode(adev->ddev);
-   }
-
-   adev->asic_reset_res = 0;
-}
-
-static void amdgpu_device_lock_adev(struct amdgpu_device *adev)
-{
-   mutex_lock(&adev->lock_reset);
atomic_inc(&adev->gpu_reset_counter);
adev->in_gpu_reset = 1;
/* Block kfd: SRIOV would do it separately */
if (!amdgpu_sriov_vf(adev))
 amdgpu_amdkfd_pre_reset(adev);
+
+   return true;
 }
 
 static void amdgpu_device_unlock_adev(struct amdgpu_device *adev)
@@ -3538,40 +3521,42 @@ static void amdgpu_device_unlock_adev(struct 
amdgpu_device *adev)
 int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
  struct amdgpu_job *job)
 {
-   int r;
+   struct list_head device_list, *device_list_handle =  NULL;
+   bool need_full_reset, job_signaled;
struct amdgpu_hive_info *hive = NULL;
-   bool need_full_reset = false;
struct amdgpu_device *tmp_adev = NULL;
-   struct list_head device_list, *device_list_handle =  NULL;
+   int i, r = 0;
 
+   need_full_reset = job_signaled = false;
INIT_LIST_HEAD(&device_list);
 
dev_info(adev->dev, "GPU reset begin!\n");
 
+   hive = amdgpu_get_xgmi_hive(adev, false);
+
/*
-* In case of XGMI hive disallow concurrent resets to be triggered
-* by different nodes. No point also since the one node already 
executing
-* reset will also reset all the other nodes in the hive.
+* Here we trylock to avoid chain of resets executing from
+* either trigger by jobs on different adevs in XGMI hive or jobs on
+* different schedulers for same device while this TO handler is 
running.
+* We always reset all schedulers for device and all devices for XGMI
+* hive so that should take care of them too.
 */
-   hive = amdgpu_get_xgmi_hive(adev, 0);
-   if (hive && adev->gmc.xgmi.num_physical_nodes > 1 &&
-   !mutex_trylock(&hive->reset_lock))
+
+   if (hive && !mutex_trylock(&hive->reset_lock)) {
+   DRM_INFO("Bailing on TDR for s_job:%llx, hive: %llx as another 
already in progress",
+job->base.id, hive->hive_id);
return 0;
+   }
 
/* Start with adev pre asic reset first for soft reset check.*/
-   amdgpu_device_lock_adev(adev);
-   r = amdgpu_device_pre_asic_reset(adev,
-job,
- 

[PATCH v5 3/6] drm/scheduler: rework job destruction

2019-04-18 Thread Andrey Grodzovsky
From: Christian König 

We now destroy finished jobs from the worker thread to make sure that
we never destroy a job currently in timeout processing.
By this we avoid holding lock around ring mirror list in drm_sched_stop
which should solve a deadlock reported by a user.

v2: Remove unused variable.
v4: Move guilty job free into sched code.
v5:
Move sched->hw_rq_count to drm_sched_start to account for counter
decrement in drm_sched_stop even when we don't call resubmit jobs
if guily job did signal.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109692

Signed-off-by: Christian König 
Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   9 +-
 drivers/gpu/drm/etnaviv/etnaviv_dump.c |   4 -
 drivers/gpu/drm/etnaviv/etnaviv_sched.c|   2 +-
 drivers/gpu/drm/lima/lima_sched.c  |   2 +-
 drivers/gpu/drm/panfrost/panfrost_job.c|   2 +-
 drivers/gpu/drm/scheduler/sched_main.c | 159 +
 drivers/gpu/drm/v3d/v3d_sched.c|   2 +-
 include/drm/gpu_scheduler.h|   6 +-
 8 files changed, 102 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7cee269..a0e165c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3334,7 +3334,7 @@ static int amdgpu_device_pre_asic_reset(struct 
amdgpu_device *adev,
if (!ring || !ring->sched.thread)
continue;
 
-   drm_sched_stop(&ring->sched);
+   drm_sched_stop(&ring->sched, &job->base);
 
/* after all hw jobs are reset, hw fence is meaningless, so 
force_completion */
amdgpu_fence_driver_force_completion(ring);
@@ -3343,8 +3343,6 @@ static int amdgpu_device_pre_asic_reset(struct 
amdgpu_device *adev,
if(job)
drm_sched_increase_karma(&job->base);
 
-
-
if (!amdgpu_sriov_vf(adev)) {
 
if (!need_full_reset)
@@ -3482,8 +3480,7 @@ static int amdgpu_do_asic_reset(struct amdgpu_hive_info 
*hive,
return r;
 }
 
-static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev,
- struct amdgpu_job *job)
+static void amdgpu_device_post_asic_reset(struct amdgpu_device *adev)
 {
int i;
 
@@ -3623,7 +3620,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 
/* Post ASIC reset for all devs .*/
list_for_each_entry(tmp_adev, device_list_handle, gmc.xgmi.head) {
-   amdgpu_device_post_asic_reset(tmp_adev, tmp_adev == adev ? job 
: NULL);
+   amdgpu_device_post_asic_reset(tmp_adev);
 
if (r) {
/* bad news, how to tell it to userspace ? */
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c 
b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
index 33854c9..5778d9c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c
@@ -135,13 +135,11 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
mmu_size + gpu->buffer.size;
 
/* Add in the active command buffers */
-   spin_lock_irqsave(&gpu->sched.job_list_lock, flags);
list_for_each_entry(s_job, &gpu->sched.ring_mirror_list, node) {
submit = to_etnaviv_submit(s_job);
file_size += submit->cmdbuf.size;
n_obj++;
}
-   spin_unlock_irqrestore(&gpu->sched.job_list_lock, flags);
 
/* Add in the active buffer objects */
list_for_each_entry(vram, &gpu->mmu->mappings, mmu_node) {
@@ -183,14 +181,12 @@ void etnaviv_core_dump(struct etnaviv_gpu *gpu)
  gpu->buffer.size,
  etnaviv_cmdbuf_get_va(&gpu->buffer));
 
-   spin_lock_irqsave(&gpu->sched.job_list_lock, flags);
list_for_each_entry(s_job, &gpu->sched.ring_mirror_list, node) {
submit = to_etnaviv_submit(s_job);
etnaviv_core_dump_mem(&iter, ETDUMP_BUF_CMD,
  submit->cmdbuf.vaddr, submit->cmdbuf.size,
  etnaviv_cmdbuf_get_va(&submit->cmdbuf));
}
-   spin_unlock_irqrestore(&gpu->sched.job_list_lock, flags);
 
/* Reserve space for the bomap */
if (n_bomap_pages) {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c 
b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 6d24fea..a813c82 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -109,7 +109,7 @@ static void etnaviv_sched_timedout_job(struct drm_sched_job 
*sched_job)
}
 
/* block scheduler */
-   drm_sched_stop(&gpu->sched);
+   drm_sched_stop(&gpu->sched, sched_job);
 
if(sched_job)
drm_sched_increase_karma(sched_job);
diff --git a/drivers/gpu/drm/lima/lima_sched.c 
b/drivers/gpu/drm/li

[PATCH v5 2/6] drm/amd/display: Use a reasonable timeout for framebuffer fence waits

2019-04-18 Thread Andrey Grodzovsky
Patch '5edb0c9b Fix deadlock with display during hanged ring recovery'
was accidentaly removed during one of DALs code merges.

v4: Update description.

Signed-off-by: Andrey Grodzovsky 
Reviewed-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

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 ad4f0e5..88e42ad 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4816,11 +4816,16 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 
abo = gem_to_amdgpu_bo(fb->obj[0]);
 
-   /* Wait for all fences on this FB */
+   /*
+* Wait for all fences on this FB. Do limited wait to avoid
+* deadlock during GPU reset when this fence will not signal
+* but we hold reservation lock for the BO.
+*/
r = reservation_object_wait_timeout_rcu(abo->tbo.resv, true,
false,
-   MAX_SCHEDULE_TIMEOUT);
-   WARN_ON(r < 0);
+   msecs_to_jiffies(5000));
+   if (unlikely(r <= 0))
+   DRM_ERROR("Waiting for fences timed out or 
interrupted!");
 
/*
 * TODO This might fail and hence better not used, wait
@@ -4829,10 +4834,8 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 * blocking commit to as per framework helpers
 */
r = amdgpu_bo_reserve(abo, true);
-   if (unlikely(r != 0)) {
+   if (unlikely(r != 0))
DRM_ERROR("failed to reserve buffer before flip\n");
-   WARN_ON(1);
-   }
 
amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 4/6] drm/sched: Keep s_fence->parent pointer

2019-04-18 Thread Andrey Grodzovsky
For later driver's reference to see if the fence is signaled.

v2: Move parent fence put to resubmit jobs.

Signed-off-by: Andrey Grodzovsky 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/scheduler/sched_main.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
b/drivers/gpu/drm/scheduler/sched_main.c
index 7816de7..03e6bd8 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -375,8 +375,6 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, struct 
drm_sched_job *bad)
if (s_job->s_fence->parent &&
dma_fence_remove_callback(s_job->s_fence->parent,
  &s_job->cb)) {
-   dma_fence_put(s_job->s_fence->parent);
-   s_job->s_fence->parent = NULL;
atomic_dec(&sched->hw_rq_count);
} else {
/*
@@ -403,6 +401,14 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, 
struct drm_sched_job *bad)
sched->ops->free_job(s_job);
}
}
+
+   /*
+* Stop pending timer in flight as we rearm it in  drm_sched_start. This
+* avoids the pending timeout work in progress to fire right away after
+* this TDR finished and before the newly restarted jobs had a
+* chance to complete.
+*/
+   cancel_delayed_work(&sched->work_tdr);
 }
 
 EXPORT_SYMBOL(drm_sched_stop);
@@ -477,6 +483,7 @@ void drm_sched_resubmit_jobs(struct drm_gpu_scheduler 
*sched)
if (found_guilty && s_job->s_fence->scheduled.context == 
guilty_context)
dma_fence_set_error(&s_fence->finished, -ECANCELED);
 
+   dma_fence_put(s_job->s_fence->parent);
s_job->s_fence->parent = sched->ops->run_job(s_job);
}
 }
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 5/6] drm/scheduler: Add flag to hint the release of guilty job.

2019-04-18 Thread Andrey Grodzovsky
Problem:
Sched thread's cleanup function races against TO handler
and removes the guilty job from mirror list and we
have no way of differentiating if the job was removed from within the
TO handler or from the sched thread's clean-up function.

Fix:
Add a flag to scheduler to hint the TO handler that the guilty job needs
to be explicitly released.

Signed-off-by: Andrey Grodzovsky 
---
 drivers/gpu/drm/scheduler/sched_main.c | 9 +++--
 include/drm/gpu_scheduler.h| 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_main.c 
b/drivers/gpu/drm/scheduler/sched_main.c
index 03e6bd8..f8f0e1c 100644
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -293,8 +293,10 @@ static void drm_sched_job_timedout(struct work_struct 
*work)
 * Guilty job did complete and hence needs to be manually removed
 * See drm_sched_stop doc.
 */
-   if (list_empty(&job->node))
+   if (sched->free_guilty) {
job->sched->ops->free_job(job);
+   sched->free_guilty = false;
+   }
 
spin_lock_irqsave(&sched->job_list_lock, flags);
drm_sched_start_timeout(sched);
@@ -395,10 +397,13 @@ void drm_sched_stop(struct drm_gpu_scheduler *sched, 
struct drm_sched_job *bad)
 
/*
 * We must keep bad job alive for later use during
-* recovery by some of the drivers
+* recovery by some of the drivers but leave a hint
+* that the guilty job must be released.
 */
if (bad != s_job)
sched->ops->free_job(s_job);
+   else
+   sched->free_guilty = true;
}
}
 
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 9ee0f27..fc0b421 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -259,6 +259,7 @@ struct drm_sched_backend_ops {
  *  guilty and it will be considered for scheduling further.
  * @num_jobs: the number of jobs in queue in the scheduler
  * @ready: marks if the underlying HW is ready to work
+ * @free_guilty: A hit to time out handler to free the guilty job.
  *
  * One scheduler is implemented for each hardware ring.
  */
@@ -279,6 +280,7 @@ struct drm_gpu_scheduler {
int hang_limit;
atomic_tnum_jobs;
boolready;
+   boolfree_guilty;
 };
 
 int drm_sched_init(struct drm_gpu_scheduler *sched,
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 1/6] drm/amd/display: wait for fence without holding reservation lock

2019-04-18 Thread Andrey Grodzovsky
From: Christian König 

Don't block others while waiting for the fences to finish, concurrent
submission is perfectly valid in this case and holding the lock can
prevent killed applications from terminating.

Signed-off-by: Christian König 
Reviewed-by: Nicholas Kazlauskas 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

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 380a7f9..ad4f0e5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4814,23 +4814,26 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
continue;
}
 
+   abo = gem_to_amdgpu_bo(fb->obj[0]);
+
+   /* Wait for all fences on this FB */
+   r = reservation_object_wait_timeout_rcu(abo->tbo.resv, true,
+   false,
+   MAX_SCHEDULE_TIMEOUT);
+   WARN_ON(r < 0);
+
/*
 * TODO This might fail and hence better not used, wait
 * explicitly on fences instead
 * and in general should be called for
 * blocking commit to as per framework helpers
 */
-   abo = gem_to_amdgpu_bo(fb->obj[0]);
r = amdgpu_bo_reserve(abo, true);
if (unlikely(r != 0)) {
DRM_ERROR("failed to reserve buffer before flip\n");
WARN_ON(1);
}
 
-   /* Wait for all fences on this FB */
-   WARN_ON(reservation_object_wait_timeout_rcu(abo->tbo.resv, 
true, false,
-   
MAX_SCHEDULE_TIMEOUT) < 0);
-
amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
 
amdgpu_bo_unreserve(abo);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110371] HP Dreamcolor display *Error* No EDID read

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110371

Michel Dänzer  changed:

   What|Removed |Added

  Component|DRM/AMDgpu  |DRM/other

-- 
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 21/29] tracing: Use percpu stack trace buffer more intelligently

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 10:41:40 +0200
Thomas Gleixner  wrote:

> The per cpu stack trace buffer usage pattern is odd at best. The buffer has
> place for 512 stack trace entries on 64-bit and 1024 on 32-bit. When
> interrupts or exceptions nest after the per cpu buffer was acquired the
> stacktrace length is hardcoded to 8 entries. 512/1024 stack trace entries
> in kernel stacks are unrealistic so the buffer is a complete waste.
> 
> Split the buffer into chunks of 64 stack entries which is plenty. This
> allows nesting contexts (interrupts, exceptions) to utilize the cpu buffer
> for stack retrieval and avoids the fixed length allocation along with the
> conditional execution pathes.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: Steven Rostedt 
> ---
>  kernel/trace/trace.c |   77 
> +--
>  1 file changed, 39 insertions(+), 38 deletions(-)
> 
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2749,12 +2749,21 @@ trace_function(struct trace_array *tr,
>  
>  #ifdef CONFIG_STACKTRACE
>  
> -#define FTRACE_STACK_MAX_ENTRIES (PAGE_SIZE / sizeof(unsigned long))
> +/* 64 entries for kernel stacks are plenty */
> +#define FTRACE_KSTACK_ENTRIES64
> +
>  struct ftrace_stack {
> - unsigned long   calls[FTRACE_STACK_MAX_ENTRIES];
> + unsigned long   calls[FTRACE_KSTACK_ENTRIES];
>  };
>  
> -static DEFINE_PER_CPU(struct ftrace_stack, ftrace_stack);
> +/* This allows 8 level nesting which is plenty */

Can we make this 4 level nesting and increase the size? (I can see us
going more than 64 deep, kernel developers never cease to amaze me ;-)
That's all we need:

 Context: Normal, softirq, irq, NMI

Is there any other way to nest?

-- Steve

> +#define FTRACE_KSTACK_NESTING(PAGE_SIZE / sizeof(struct 
> ftrace_stack))
> +
> +struct ftrace_stacks {
> + struct ftrace_stack stacks[FTRACE_KSTACK_NESTING];
> +};
> +
> +static DEFINE_PER_CPU(struct ftrace_stacks, ftrace_stacks);
>  static DEFINE_PER_CPU(int, ftrace_stack_reserve);
>  
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 28/29] stacktrace: Provide common infrastructure

2019-04-18 Thread Josh Poimboeuf
On Thu, Apr 18, 2019 at 10:41:47AM +0200, Thomas Gleixner wrote:
> All architectures which support stacktrace carry duplicated code and
> do the stack storage and filtering at the architecture side.
> 
> Provide a consolidated interface with a callback function for consuming the
> stack entries provided by the architecture specific stack walker. This
> removes lots of duplicated code and allows to implement better filtering
> than 'skip number of entries' in the future without touching any
> architecture specific code.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: linux-a...@vger.kernel.org

This is a step in the right direction, especially if it allows us to get
rid of the 'skip' stuff.  But I'm not crazy about the callbacks.

Another idea I had (but never got a chance to work on) was to extend the
x86 unwind interface to all arches.  So instead of the callbacks, each
arch would implement something like this API:


struct unwind_state state;

void unwind_start(struct unwind_state *state, struct task_struct *task,
  struct pt_regs *regs, unsigned long *first_frame);

bool unwind_next_frame(struct unwind_state *state);

inline bool unwind_done(struct unwind_state *state);


Not only would it avoid the callbacks (which is a nice benefit already),
it would also allow the interfaces to be used outside of the
stack_trace_*() interfaces.  That would come in handy in cases like the
ftrace stack tracer code, which needs more than the stack_trace_*() API
can give.

Of course, this may be more work than what you thought you signed up for
;-)

-- 
Josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 1/1] drm/i915: Use drm_dev_unplug()

2019-04-18 Thread Janusz Krzysztofik
From: Janusz Krzysztofik 

The driver does not currently support unbinding from a device which is
in use.  Since open file descriptors may still be pointing into kernel
memory where the device structures used to be, entirely correct kernel
panics protect the driver from being unbound as we should not be
unbinding it before those dangling pointers have been made safe.

According to the documentation found inside drivers/gpu/drm/drm_drv.c,
drm_dev_unplug() should be used instead of drm_dev_unregister() in
order to make a device inaccessible to users as soon as it is unpluged.
Follow that advice to make those possibly dangling pointers safe,
protected by DRM layer from a user who is otherwise left pointing into
possibly reused kernel memory after the driver has been unbound from
the device.

Signed-off-by: Janusz Krzysztofik 
Reviewed-by: Chris Wilson 
Reviewed-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9df65d386d11..66163378c481 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1596,7 +1596,7 @@ static void i915_driver_unregister(struct 
drm_i915_private *dev_priv)
i915_pmu_unregister(dev_priv);
 
i915_teardown_sysfs(dev_priv);
-   drm_dev_unregister(&dev_priv->drm);
+   drm_dev_unplug(&dev_priv->drm);
 
i915_gem_shrinker_unregister(dev_priv);
 }
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 0/1] Stop users from using the device on driver unbind

2019-04-18 Thread Janusz Krzysztofik
Use drm_dev_unplug() to have device resources protected from user access
by DRM layer as soon as the driver is going to be unbound.

Janusz Krzysztofik (1):
  drm/i915: Use drm_dev_unplug()

 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Since this patch should be now safe for use if merged with current
drm-next or drm-tip branch which no longer suffer from incorrectly
resolved merge confilct that was breaking it, finally fixed by commit
bd53280ef042 ("drm/drv: Fix incorrect resolution of merge conflict"),
I'm resending it with Daniel's Reviewed-by: added.

Former patch 2/2 has been dropped as it is already in drm-intel-next as
commit 141f3767e7b8 ("drm/i915: Mark GEM wedged right after marking
device unplugged").  BTW, the wersion I sent was screwed up, not
reflecting Chris' intention precisely enough, but Chris was vigilant and
fixed it.  Sorry Chris.

Thanks,
Janusz
-- 
2.20.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110371] HP Dreamcolor display *Error* No EDID read

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110371

--- Comment #3 from Nicholas Kazlauskas  ---
Looks like this is a bug in DRM itself with the parsing.

[1.325111] [drm] parse error at position 12 in video mode '1920x1080@59.94'
[1.335096] [drm:dc_link_detect [amdgpu]] *ERROR* No EDID read.

It's hitting the "." as part of the video mode and erroring out because DRM
doesn't consider it a valid character.

-- 
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: Improvements to VRR below-the-range/low framerate compensation.

2019-04-18 Thread Kazlauskas, Nicholas
On 4/18/19 10:24 AM, Michel Dänzer wrote:
> On 2019-04-18 5:51 a.m., Mario Kleiner wrote:
>>
>> My desired application of VRR for neuroscience/vision research
>> is to control the timing of when frames show up onscreen, e.g.,
>> to show animations at different "unconventional" framerates,
>> so i'm mostly interested in how well one can control the timing
>> between successive OpenGL bufferswaps. This is a bit different
>> from what a game wants to get out of VRR, probably closer to
>> what a movie player might want to do.
> 
> As discussed before, I expect that games which care about smooth
> animation will want to control the presentation time as well in the long
> run, so that they can make the presentation time match the simulation
> time corresponding to the frame contents.
> 
> There is already support for specifying the target presentation time in
> (at least, there might be more I'm not aware of) VDPAU, the Vulkan
> VK_GOOGLE_display_timing extension, and the X11 Present extension
> protocol (not actually implemented yet in xserver though).
> 
> Ideally, this should also be available in the kernel UAPI. That should
> allow hitting the target more accurately / reliably, and might also
> allow making better use of compensation mechanisms such as those touched
> by this series, since userspace can call into the kernel ahead of the
> target time, giving the kernel an opportunity to make adjustments earlier.
> 
> Since you have a strong use-case for this functionality, maybe you'd be
> interested in working on it?
> 
> 

This would be a good real-world userspace application that takes 
advantage of this.

There were some threads before in the original VRR RFC and patch series 
that discussion this, but I think the summary was that most people 
interested were in favor of a timestamp in nanoseconds on the CRTC.

My thoughts on the matter would be that the driver would be enforced not 
to present the frame any earlier than the timestamp given. If the driver 
isn't capable of doing that then it can return -EINVAL (ie. userspace 
asks for a really large timestamp and the driver would have to stall 
forever in order to display it).

There could still be some leeway in terms of when the frame could be 
displayed. Maybe another property could be also used to specify the 
upper bound for presentation.

I would imagine any properties created here would be optional to drivers 
to add. In hindsight, the VRR CRTC property should probably have been 
optional as well.

Nicholas Kazlauskas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drm/panfrost: depend on !GENERIC_ATOMIC64 when using COMPILE_TEST

2019-04-18 Thread Rob Herring
On Wed, Apr 17, 2019 at 10:29 AM Steven Price  wrote:
>
> Since panfrost has a 'select' on IOMMU_IO_PGTABLE_LPAE we must depend on
> the same set of flags. Otherwise IOMMU_IO_PGTABLE_LPAE will be forced on
> even though it cannot build (no support for cmpxchg64).
>
> This fixes the following warning from kconfig:
>
> WARNING: unmet direct dependencies detected for IOMMU_IO_PGTABLE_LPAE
>   Depends on [n]: IOMMU_SUPPORT [=y] && (ARM || ARM64 || COMPILE_TEST [=y] && 
> !GENERIC_ATOMIC64 [=y])
>   Selected by [y]:
>   - DRM_PANFROST [=y] && HAS_IOMEM [=y] && DRM [=y] && (ARM || ARM64 || 
> COMPILE_TEST [=y]) && MMU [=y]
>
> Reported-by: kbuild test robot 
> Signed-off-by: Steven Price 
> ---
>  drivers/gpu/drm/panfrost/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Humm, I thought I had fixed this. Anyways, applied.

Rob
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/panfrost: Prevent concurrent resets

2019-04-18 Thread Rob Herring
On Thu, Apr 18, 2019 at 3:43 AM Tomeu Vizoso  wrote:
>
> If a job times out in slot 0 while a reset is performed because a job
> timed out in slot 1, the drm-sched core can get into a deadlock.
>
> Signed-off-by: Tomeu Vizoso 
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 1 +
>  drivers/gpu/drm/panfrost/panfrost_device.h | 1 +
>  drivers/gpu/drm/panfrost/panfrost_job.c| 4 
>  3 files changed, 6 insertions(+)

Applied, thanks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drm/panfrost: Make panfrost_gem_free_object() static

2019-04-18 Thread Rob Herring
On Wed, Apr 17, 2019 at 9:51 AM Steven Price  wrote:
>
> On 16/04/2019 16:00, Yue Haibing wrote:
> > From: YueHaibing 
> >
> > Fix sparse warning:
> >
> > drivers/gpu/drm/panfrost/panfrost_gem.c:17:6:
> >  warning: symbol 'panfrost_gem_free_object' was not declared. Should it be 
> > static?
> >
> > Reported-by: Hulk Robot 
> > Signed-off-by: YueHaibing 
>
> Reviewed-by: Steven Price 
>
> Although while we're fixing sparse warnings, there's a few more in Panfrost:
>
> -8<---
> From 8aaf778262744cfbebb9b7f274ead9ba600526b0 Mon Sep 17 00:00:00 2001
> From: Steven Price 
> Date: Wed, 17 Apr 2019 15:47:49 +0100
> Subject: [PATCH] drm/panfrost: Add missing include
>
> Fix sparse warnings:
> drivers/gpu/drm/panfrost/panfrost_devfreq.c:133:5:
>  warning: symbol 'panfrost_devfreq_init' was not declared. Should it be 
> static?
> drivers/gpu/drm/panfrost/panfrost_devfreq.c:168:6:
>  warning: symbol 'panfrost_devfreq_resume' was not declared. Should it be 
> static?
> drivers/gpu/drm/panfrost/panfrost_devfreq.c:182:6:
>  warning: symbol 'panfrost_devfreq_suspend' was not declared. Should it be 
> static?
> drivers/gpu/drm/panfrost/panfrost_devfreq.c:212:6:
>  warning: symbol 'panfrost_devfreq_record_transition' was not declared. 
> Should it be static?
>
> Signed-off-by: Steven Price 
> ---
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH -next] drm/panfrost: Make panfrost_gem_free_object() static

2019-04-18 Thread Rob Herring
On Tue, Apr 16, 2019 at 10:01 AM Yue Haibing  wrote:
>
> From: YueHaibing 
>
> Fix sparse warning:
>
> drivers/gpu/drm/panfrost/panfrost_gem.c:17:6:
>  warning: symbol 'panfrost_gem_free_object' was not declared. Should it be 
> static?
>
> Reported-by: Hulk Robot 
> Signed-off-by: YueHaibing 
> ---
>  drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] sun4i/drm: Fixes for removing and re-probing the driver

2019-04-18 Thread Maxime Ripard
On Thu, Apr 18, 2019 at 03:27:24PM +0200, Paul Kocialkowski wrote:
> This series brings-in some fixes that are necessary to be able to
> remove the driver at run-time (when built as a module) and properly
> re-probe it afterwards.

Applied all three, thanks
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/sun4i: Use DRM_GEM_CMA_VMAP_DRIVER_OPS for GEM operations

2019-04-18 Thread Maxime Ripard
On Thu, Apr 18, 2019 at 03:05:09PM +0200, Paul Kocialkowski wrote:
> Our driver makes a typical use of CMA, with GEM object allocated as
> GEM CMA objects. Use DRM_GEM_CMA_VMAP_DRIVER_OPS to describe the ops
> instead of duplicating them.
>
> Because DRM_GEM_CMA_VMAP_DRIVER_OPS implements a gem_create_object op
> which sets per-object funcs (drm_cma_gem_default_funcs), we can also
> get rid of free_object_unlocked and gem_vm_ops, which are superseded
> by the object funcs.
>
> Signed-off-by: Paul Kocialkowski 

Applied, thanks

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Improvements to VRR below-the-range/low framerate compensation.

2019-04-18 Thread Michel Dänzer
On 2019-04-18 5:51 a.m., Mario Kleiner wrote:
> 
> My desired application of VRR for neuroscience/vision research
> is to control the timing of when frames show up onscreen, e.g.,
> to show animations at different "unconventional" framerates,
> so i'm mostly interested in how well one can control the timing
> between successive OpenGL bufferswaps. This is a bit different
> from what a game wants to get out of VRR, probably closer to
> what a movie player might want to do.

As discussed before, I expect that games which care about smooth
animation will want to control the presentation time as well in the long
run, so that they can make the presentation time match the simulation
time corresponding to the frame contents.

There is already support for specifying the target presentation time in
(at least, there might be more I'm not aware of) VDPAU, the Vulkan
VK_GOOGLE_display_timing extension, and the X11 Present extension
protocol (not actually implemented yet in xserver though).

Ideally, this should also be available in the kernel UAPI. That should
allow hitting the target more accurately / reliably, and might also
allow making better use of compensation mechanisms such as those touched
by this series, since userspace can call into the kernel ahead of the
target time, giving the kernel an opportunity to make adjustments earlier.

Since you have a strong use-case for this functionality, maybe you'd be
interested in working on it?


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109456] KVM VFIO guest X hang with guest kernel > 4.15

2019-04-18 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=109456

--- Comment #2 from libgra...@gmail.com ---
Created attachment 144034
  --> https://bugs.freedesktop.org/attachment.cgi?id=144034&action=edit
Git bisect log + result

-- 
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/amdgpu: fix drm leases being broken on radv

2019-04-18 Thread Emil Velikov
On Wed, 17 Apr 2019 at 21:49, Dave Airlie  wrote:
>
> On Thu, 18 Apr 2019 at 03:30, Koenig, Christian
>  wrote:
> >
> > Am 17.04.19 um 17:51 schrieb Emil Velikov:
> > > Hi guys,
> > >
> > > On 2019/04/17, Daniel Vetter wrote:
> > >> On Wed, Apr 17, 2019 at 02:46:06PM +0200, Christian König wrote:
> > >>> Am 17.04.19 um 14:35 schrieb Daniel Vetter:
> >  On Wed, Apr 17, 2019 at 12:06:32PM +, Koenig, Christian wrote:
> > > Am 17.04.19 um 14:00 schrieb Daniel Vetter:
> > >> On Wed, Apr 17, 2019 at 11:18:35AM +, Koenig, Christian wrote:
> > >>> Am 17.04.19 um 13:06 schrieb Daniel Vetter:
> >  On Wed, Apr 17, 2019 at 12:29 PM Koenig, Christian
> >   wrote:
> >  [SNIP]
> > >>> Well, what you guys did here is a serious no-go.
> > >> Not really understanding your reasons for an unconditional nak on 
> > >> all this
> > >> still.
> > > Well, let me summarize: You worked around a problem with libva by
> > > breaking another driver and now proposing a rather ugly and only halve
> > > backed workaround for that driver.
> > >
> > > This is a serious no-go. I've already send out a i915 specific change 
> > > to
> > > remove DRM_AUTH from IOCTLs which also have DRM_RENDER_ALLOW and 
> > > revert
> > > the offending patch.
> >  Oh I'm totally fine with the revert if that's what we go with. But that
> >  still leaves the issue that it would make sense to drop DRM_AUTH from
> >  DRIVER_RENDER capable drivers (not just for libva, but in general). And
> >  there's fundamentally 2 options to do that:
> > 
> >  - This one here (or anything implementing the same idea), to keep radv
> >  working.
> > 
> >  - We drop DRM_AUTH from all drivers with DRIVER_RENDER except amdgpu. 
> >  How
> >  exactly that's doen doesn't matter, but it leaves amdgpu out in the
> >  cold. It also doesn't matter whether we get there with revert + 
> >  lots of
> >  patches, or a special hack for amdgpu, in the end amdgpu would be
> >  different. Also note that your patch isn't enough, since it 
> >  doesn't fix
> >  the core ioctls.
> > 
> >  I think from an overall platform pov, the first is the better option.
> >  After all we try really hard to avoid driver special cases for these 
> >  kinds
> >  of things.
> > >>> Well, I initially thought that radv is doing something unusual or 
> > >>> broken,
> > >>> but after looking deeper into this that is actually not the case.
> > >>>
> > >>> What radv does is calling an IOCTL and expecting an error message when 
> > >>> it is
> > >>> not authenticated.
> > >>>
> > >>> It looks like libdrm_amdgpu has switched to using DRM_IOCTL_GET_CLIENT 
> > >>> to
> > >>> figure out if it is authenticated or not, but I clearly remember that we
> > >>> haven't done this from the beginning.
> > >> Maybe in the radoen code, or some earlier internal amdgpu libdrm code?
> > >> First public commit has all the bits: getauth, GetVersion, then the accel
> > >> query.
> > >>
> > >>> Thinking more about this I don't think that this problem is actually 
> > >>> amdgpu
> > >>> specific. So I wouldn't drop DRM_AUTH from all render node drivers at 
> > >>> all,
> > >>> but only from those who have the specific issue with libva.
> > >> libva was the initial motivation, the goal of Emil's patch wasn't to just
> > >> duct-tape over libva. That would be easier to fix in libva. The point was
> > >> that we should be able to allow this in general.
> > >>
> > >> And we can, except for the radv issue uncovered here.
> > >>
> > >> So please don't get 100% hung up on the libva thing, that wasn't really
> > >> the goal, just the initial motivation. And I still thinks it makes for 
> > >> all
> > >> drivers. So again we have:
> > >>
> > >> - radv hack
> > >> - make amdgpu behave different from everyone else
> > >> - keep tilting windmills about "pls use rendernodes, thx"
> > >>
> > >> I neither like tilting windmills nor making drivers behave different for
> > >> no reaason at all.
> > > Allow me to jump-in some emails down the line.
> > >
> > > First and foremost, sincere apologies for upsetting you Christian. If
> > > it's of any consolidation - let me assure you the goal is _not_ to break
> > > amdgpu or any other driver.
> > >
> > > Secondly, I would like to ask you for a list of projects so we can look 
> > > and
> > > investigate properly. So far you've mentioned libdrm_amdgpu - I'll look 
> > > into
> > > it.
> >
> > That is rather hard to come by, because you would also need to look at
> > all previously existing driver stacks.
> >
> > E.g. with the classic R300 driver for example.
> >
> > > On the topic of "working around libva" - sadly libva is _not_ the only
> > > offender. We also had bugs in mesa and kmscube.
> > >
> > > Considering those are taken as a prime example of "the right way", it's 
> > > very
> > > likely for the mi

Re: [patch V2 01/29] tracing: Cleanup stack trace code

2019-04-18 Thread Josh Poimboeuf
On Thu, Apr 18, 2019 at 10:41:20AM +0200, Thomas Gleixner wrote:
> - Remove the extra array member of stack_dump_trace[]. It's not required as
>   the stack tracer stores at max array size - 1 entries so there is still
>   an empty slot.

What is the empty slot used for?

-- 
Josh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 20/29] tracing: Simplify stacktrace retrieval in histograms

2019-04-18 Thread Steven Rostedt

[ Added Tom Zanussi ]

On Thu, 18 Apr 2019 10:41:39 +0200
Thomas Gleixner  wrote:

> The indirection through struct stack_trace is not necessary at all. Use the
> storage array based interface.
> 
> Signed-off-by: Thomas Gleixner 
> Cc: Steven Rostedt 

Looks fine to me

Acked-by: Steven Rostedt (VMware) 

 But...

Tom,

Can you review this too?

Patch series starts here:

  http://lkml.kernel.org/r/20190418084119.056416...@linutronix.de

Thanks,

-- Steve

> ---
>  kernel/trace/trace_events_hist.c |   12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -5186,7 +5186,6 @@ static void event_hist_trigger(struct ev
>   u64 var_ref_vals[TRACING_MAP_VARS_MAX];
>   char compound_key[HIST_KEY_SIZE_MAX];
>   struct tracing_map_elt *elt = NULL;
> - struct stack_trace stacktrace;
>   struct hist_field *key_field;
>   u64 field_contents;
>   void *key = NULL;
> @@ -5198,14 +5197,9 @@ static void event_hist_trigger(struct ev
>   key_field = hist_data->fields[i];
>  
>   if (key_field->flags & HIST_FIELD_FL_STACKTRACE) {
> - stacktrace.max_entries = HIST_STACKTRACE_DEPTH;
> - stacktrace.entries = entries;
> - stacktrace.nr_entries = 0;
> - stacktrace.skip = HIST_STACKTRACE_SKIP;
> -
> - memset(stacktrace.entries, 0, HIST_STACKTRACE_SIZE);
> - save_stack_trace(&stacktrace);
> -
> + memset(entries, 0, HIST_STACKTRACE_SIZE);
> + stack_trace_save(entries, HIST_STACKTRACE_DEPTH,
> +  HIST_STACKTRACE_SKIP);
>   key = entries;
>   } else {
>   field_contents = key_field->fn(key_field, elt, rbe, 
> rec);
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [patch V2 14/29] dm bufio: Simplify stack trace retrieval

2019-04-18 Thread Steven Rostedt
On Thu, 18 Apr 2019 14:11:44 +0200
Alexander Potapenko  wrote:

> On Thu, Apr 18, 2019 at 1:54 PM Thomas Gleixner  wrote:
> >
> > On Thu, 18 Apr 2019, Alexander Potapenko wrote:  
> > > On Thu, Apr 18, 2019 at 11:06 AM Thomas Gleixner  
> > > wrote:  
> > > > -   save_stack_trace(&b->stack_trace);
> > > > +   b->stack_len = stack_trace_save(b->stack_entries, MAX_STACK, 
> > > > 2);  
> > > As noted in one of similar patches before, can we have an inline
> > > comment to indicate what does this "2" stand for?  
> >
> > Come on. We have gazillion of functions which take numerical constant
> > arguments. Should we add comments to all of them?  
> Ok, sorry. I might not be familiar enough with the kernel style guide.

It is a legitimate complaint but not for this series. I only complain
about hard coded constants when they are added. That "2" was not
added by this series. This patch set is a clean up of the stack tracing
code, not a clean up of removing hard coded constants, or commenting
them.

The hard coded "2" was there without a comment before this patch series
and Thomas is correct to leave it as is for these changes. This patch
series should not modify what was already there which is out of scope
for the purpose of these changes.

A separate clean up patch to the maintainer of the subsystem (dm bufio
in this case) is fine. But it's not Thomas's responsibility.

-- Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/sun4i: Set device driver data at bind time for use in unbind

2019-04-18 Thread Paul Kocialkowski
Our sun4i_drv_unbind gets the drm device using dev_get_drvdata.
However, that driver data is never set in sun4i_drv_bind.

Set it there to avoid getting a NULL pointer at unbind time.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index ef38ea0a748d..af07291544a4 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -72,6 +72,8 @@ static int sun4i_drv_bind(struct device *dev)
ret = -ENOMEM;
goto free_drm;
}
+
+   dev_set_drvdata(dev, drm);
drm->dev_private = drv;
INIT_LIST_HEAD(&drv->frontend_list);
INIT_LIST_HEAD(&drv->engine_list);
-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/3] drm/sun4i: Fix component unbinding and component master deletion

2019-04-18 Thread Paul Kocialkowski
For our component-backed driver to be properly removed, we need to
delete the component master in sun4i_drv_remove and make sure to call
component_unbind_all in the master's unbind so that all components are
unbound when the master is.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index af07291544a4..0ea365e54de1 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -137,6 +137,8 @@ static void sun4i_drv_unbind(struct device *dev)
drm_mode_config_cleanup(drm);
of_reserved_mem_device_release(dev);
drm_dev_put(drm);
+
+   component_unbind_all(dev, NULL);
 }
 
 static const struct component_master_ops sun4i_drv_master_ops = {
@@ -385,6 +387,8 @@ static int sun4i_drv_probe(struct platform_device *pdev)
 
 static int sun4i_drv_remove(struct platform_device *pdev)
 {
+   component_master_del(&pdev->dev, &sun4i_drv_master_ops);
+
return 0;
 }
 
-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/3] sun4i/drm: Fixes for removing and re-probing the driver

2019-04-18 Thread Paul Kocialkowski
This series brings-in some fixes that are necessary to be able to
remove the driver at run-time (when built as a module) and properly
re-probe it afterwards.

Cheers,

Paul

Paul Kocialkowski (3):
  drm/sun4i: Add missing drm_atomic_helper_shutdown at driver unbind
  drm/sun4i: Set device driver data at bind time for use in unbind
  drm/sun4i: Fix component unbinding and component master deletion

 drivers/gpu/drm/sun4i/sun4i_drv.c | 8 
 1 file changed, 8 insertions(+)

-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/sun4i: Add missing drm_atomic_helper_shutdown at driver unbind

2019-04-18 Thread Paul Kocialkowski
A call to drm_atomic_helper_shutdown is required to properly release
the internal references taken by the core and avoid warnings about
leaking objects. Add it since it was missing.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index c8c0ab3da972..ef38ea0a748d 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -16,6 +16,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -130,6 +131,7 @@ static void sun4i_drv_unbind(struct device *dev)
 
drm_dev_unregister(drm);
drm_kms_helper_poll_fini(drm);
+   drm_atomic_helper_shutdown(drm);
drm_mode_config_cleanup(drm);
of_reserved_mem_device_release(dev);
drm_dev_put(drm);
-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2] drm: Fire off KMS hotplug events if probe detect says the connector is connected

2019-04-18 Thread Gwan-gyeong Mun
The hotplug detection routine of drm_helper_hpd_irq_event() can detect
changing of status of connector, but it can not detect changing of
properties of the connector.
e.g. changing of edid while suspend/resume, changing of dp lanes in dp aux.

Following scenario explains one of them; A detection of changing of edid.

 1) plug display device to a connector
 2) system suspend
 3) unplug 1)'s display device and plug the other display device to a
connector
 4) system resume

To solve explained cases, It fires off KMS hotplug events if
drm_helper_probe_detect() says the connector is connected.

Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend

v2: Remove suggested-by line (danvet)
Signed-off-by: Gwan-gyeong Mun 
Link: https://lists.freedesktop.org/archives/dri-devel/2019-April/214572.html
---
 drivers/gpu/drm/drm_probe_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 6fd08e04b323..081a849104f2 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -780,7 +780,8 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
  connector->name,
  drm_get_connector_status_name(old_status),
  drm_get_connector_status_name(connector->status));
-   if (old_status != connector->status)
+   if (old_status != connector->status ||
+   connector->status == connector_status_connected)
changed = true;
}
drm_connector_list_iter_end(&conn_iter);
-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fire off KMS hotplug events if probe detect says the connector is connected

2019-04-18 Thread Mun, Gwan-gyeong
On Thu, 2019-04-18 at 10:28 +0200, Daniel Vetter wrote:
> On Thu, Apr 18, 2019 at 11:09:29AM +0300, Gwan-gyeong Mun wrote:
> > The hotplug detection routine of drm_helper_hpd_irq_event() can
> > detect
> > changing of status of connector, but it can not detect changing of
> > properties of the connector.
> > e.g. changing of edid while suspend/resume, changing of dp lanes in
> > dp aux.
> > 
> > Following scenario explains one of them; A detection of changing of
> > edid.
> > 
> >  1) plug display device to a connector
> >  2) system suspend
> >  3) unplug 1)'s display device and plug the other display device to
> > a
> > connector
> >  4) system resume
> > 
> > To solve explained cases, It fires off KMS hotplug events if
> > drm_helper_probe_detect() says the connector is connected.
> > 
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> > Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
> > 
> > Suggested-by: Daniel Vetter 
> 
> This isn't at all what I suggested.
> -Daniel
Because the code modification was followed by your comments, so I added
suggested-by line. I will remove this line and will resend it.
Br,
G.G.
> 
> > Signed-off-by: Gwan-gyeong Mun 
> > Link: 
> > https://lists.freedesktop.org/archives/dri-devel/2019-April/214572.html
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c
> > b/drivers/gpu/drm/drm_probe_helper.c
> > index 6fd08e04b323..081a849104f2 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -780,7 +780,8 @@ bool drm_helper_hpd_irq_event(struct drm_device
> > *dev)
> >   connector->name,
> >   drm_get_connector_status_name(old_status)
> > ,
> >   drm_get_connector_status_name(connector-
> > >status));
> > -   if (old_status != connector->status)
> > +   if (old_status != connector->status ||
> > +   connector->status == connector_status_connected)
> > changed = true;
> > }
> > drm_connector_list_iter_end(&conn_iter);
> > -- 
> > 2.21.0
> > 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v3 0/7] Add Multi Segment Gamma Support

2019-04-18 Thread Ville Syrjälä
On Thu, Apr 18, 2019 at 09:13:04AM +0200, Daniel Vetter wrote:
> On Wed, Apr 17, 2019 at 02:57:31PM +0300, Ville Syrjälä wrote:
> > On Wed, Apr 17, 2019 at 09:28:19AM +0200, Daniel Vetter wrote:
> > > On Fri, Apr 12, 2019 at 03:50:56PM +0530, Uma Shankar wrote:
> > > > This series adds support for programmable gamma modes and
> > > > exposes a property interface for the same. Also added,
> > > > support for multi segment gamma mode introduced in ICL+
> > > >
> > > > It creates GAMMA_MODE property interface. This is an enum
> > > > property with values as blob_id's and exposes
> > > > the various gamma modes supported and the lut ranges  Getting the
> > > > blob id in userspace, user can get the mode supported and
> > > > also the range of gamma mode supported with number of lut
> > > > coefficients. It can then set one of the modes using this
> > > > enum property.
> > > >
> > > > Lut values will be sent through already available GAMMA_LUT
> > > > blob property.
> > > >
> > > > It also introduces a CLIENT CAP for advanced GAMMA_MODE.
> > > >  This is for user to set the and use advance gamma mode and older
> > > > userspace can continue using the legacy paths.
> > > >
> > > > v2: Used Ville's design and approach to define the interfaces.
> > > > Addressed Matt Roper's review feedback and re-ordered the
> > > > patches.
> > > >
> > > > v3: Converged to 1 property interface and introduced a Client cap
> > > > as suggested by Ville. Fixed review comments received.
> > > >
> > > > Uma Shankar (5):
> > > >   drm/i915/icl: Add register definitions for Multi Segmented gamma
> > > >   drm/i915/icl: Add support for multi segmented gamma mode
> > > >   drm/i915: Attach gamma mode property
> > > >   drm: Add Client Cap for advance gamma mode
> > > >   drm/i915: Enable advance gamma mode
> > > >
> > > > Ville Syrjälä (2):
> > > >   drm: Add gamma mode property
> > > >   drm/i915: Define color lut range structure
> > >
> > > Bunch of higher level comments after some internal discussions:
> > >
> > > - we need the userspace for this, can't design new uapi without involving
> > >   the compositor folks for hdr.
> > >
> > > - single property doesn't work: Once userspace has set it, the old blob
> > >   property with the list of all options is gone. We need one read-only
> > >   property for the list of options, plus a 2nd property that userspace can
> > >   set. This is a general rule for more complex properties, where the usual
> > >   property metadata isn't enough to describe the possible options.
> >
> > I guess no one understood my blob_enum idea? It's an enum where each
> > possible value is a blob. The only thing that changes is the current
> > value (which can only point to one of the enumerated blobs).
> 
> Uh yes that's not clear at all, and if we do go with this, I guess we
> should have a pile of core code to make sure it validates and is
> consistent.
> 
> >> > - no caps for properties. Yes that gives us a theoretical problem, no in
> > >   practice it doesn't matter, since people don't even care enough to make
> > >   e.g. fbdev resetting work today for everything. Long form discussion,
> > >   see here:
> > >
> > >   https://blog.ffwll.ch/2016/01/vt-switching-with-atomic-modeset.html
> > >
> > >   Nothing happened in this area ever since I typed this up, so I guess
> > >   it's really not a real-world concern.
> > >
> > > - Simplest path forward would be if we accept different LUT sizes than the
> > >   one advertised (we already do that for legacy gamma, and this is
> > >   officially what we had in mind too), and the kernel automatically picks
> > >   the best lut configuration. Will be somewhat awkard for the
> > >   multi-segment lut, but would decouple the uapi discussion a bit.
> >
> > It'll be ridiculously wasteful. IIRC we need a LUT with 32768 entries,
> > and then ~98% of those gets thrown away and never programmed to the
> > hardware.
> 
> Yeah it's a few MB, not that awesome really ...
> 
> > > - Frankly the uapi proposed looks like fake generic - it tries to model
> > >   all possibilities in a generic way, when really userspace needs to have
> > >   special code for special pipelines.
> >
> > I think it can be used pretty easily. Userspace just has to decide
> > whether it wants a straight up LUT or whether an interpolated curve
> > is enough, and how much precision it needs. For x11 the logic would
> > be simple enough: 1. look for straight up LUT with num_entries >= 1< > if that isn't found fall back to an interpolated curve with >= 1< > precision, and finally just fall back to whatever gives the best
> > results I suppose.
> 
> Hm, there's also a bunch more defines about mirroring and non-negative and
> other stuff that I have no idea how userspace should use it. I do think
> some "here's the possible configs for color management" thing is needed,
> I'm not sure userspace can do much with all the details provided in the
> current series.

The negative values would represent out of gamut col

[PATCH v2] drm/sun4i: Use DRM_GEM_CMA_VMAP_DRIVER_OPS for GEM operations

2019-04-18 Thread Paul Kocialkowski
Our driver makes a typical use of CMA, with GEM object allocated as
GEM CMA objects. Use DRM_GEM_CMA_VMAP_DRIVER_OPS to describe the ops
instead of duplicating them.

Because DRM_GEM_CMA_VMAP_DRIVER_OPS implements a gem_create_object op
which sets per-object funcs (drm_cma_gem_default_funcs), we can also
get rid of free_object_unlocked and gem_vm_ops, which are superseded
by the object funcs.

Signed-off-by: Paul Kocialkowski 
---
Changes since v1:
* Made sure our dumb_create overwrites the OPS's dumb_create.

 drivers/gpu/drm/sun4i/sun4i_drv.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c 
b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 3ebd9f5e2719..c8c0ab3da972 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -52,22 +52,8 @@ static struct drm_driver sun4i_drv_driver = {
.minor  = 0,
 
/* GEM Operations */
+   DRM_GEM_CMA_VMAP_DRIVER_OPS,
.dumb_create= drm_sun4i_gem_dumb_create,
-   .gem_free_object_unlocked = drm_gem_cma_free_object,
-   .gem_vm_ops = &drm_gem_cma_vm_ops,
-
-   /* PRIME Operations */
-   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
-   .gem_prime_import   = drm_gem_prime_import,
-   .gem_prime_export   = drm_gem_prime_export,
-   .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
-   .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
-   .gem_prime_vmap = drm_gem_cma_prime_vmap,
-   .gem_prime_vunmap   = drm_gem_cma_prime_vunmap,
-   .gem_prime_mmap = drm_gem_cma_prime_mmap,
-
-   /* Frame Buffer Operations */
 };
 
 static int sun4i_drv_bind(struct device *dev)
-- 
2.21.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix timestamp docs for variable refresh properties.

2019-04-18 Thread Kazlauskas, Nicholas
On 4/18/19 2:01 AM, Mario Kleiner wrote:
> As discussed with Nicholas and Daniel Vetter (patchwork
> link to discussion below), the VRR timestamping behaviour
> produced utterly useless and bogus vblank/pageflip
> timestamps. We have found a way to fix this and provide
> sane behaviour.
> 
> As of Linux 5.2, the amdgpu driver will be able to
> provide exactly the same vblank / pageflip timestamp
> semantic in variable refresh rate mode as in standard
> fixed refresh rate mode. This is achieved by deferring
> core vblank handling (drm_crtc_handle_vblank()) until
> the end of front porch, and also defer the sending of
> pageflip completion events until end of front porch,
> when we can safely compute correct pageflip/vblank
> timestamps.
> 
> The same approach will be possible for other VRR
> capable kms drivers, so we can actually have sane
> and useful timestamps in VRR mode.
> 
> This patch removes the section of the docs that
> describes the broken timestamp behaviour present
> in Linux 5.0/5.1.
> 
> Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> Link: https://patchwork.freedesktop.org/patch/285333/
> Signed-off-by: Mario Kleiner 

Reviewed-by: Nicholas Kazlauskas 

Someone else can feel free to push this as I don't have commit rights 
for DRM.

Thanks!

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/drm_connector.c | 6 --
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..b34c3d38bf15 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>*
>*  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.
>*/
>   
>   /**
> 

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Avoiding merge conflicts while adding new docs - Was: Re: [PATCH 00/57] Convert files to ReST

2019-04-18 Thread Mauro Carvalho Chehab
Jon,

Em Mon, 15 Apr 2019 23:55:25 -0300
Mauro Carvalho Chehab  escreveu:

> I have a separate patch series with do the actual rename and
> adjustment of references. I opted to submit this first, as it
> sounds easier to merge this way, as each subsystem maintainer
> can apply the conversion directly on their trees (or at docs
> tree), avoiding merge conflects.

Based on the number of feedbacks we have about this, I'm
considering to submit a second version of my conversion patch
series that would be doing the rename together with each patch,
adding the rst files to an index file.

However, doing that would produce lots of warnings. We have
already lots of those at linux-next:

checking consistency... 

docs/Documentation/accelerators/ocxl.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/admin-guide/mm/numaperf.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/filesystems/ext4/allocators.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/attributes.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/bigalloc.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/bitmaps.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/blockgroup.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/blocks.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/filesystems/ext4/checksums.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/directory.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/eainode.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/group_descr.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/ifork.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/filesystems/ext4/inlinedata.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/inodes.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/filesystems/ext4/journal.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/filesystems/ext4/mmp.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/filesystems/ext4/special_inodes.rst: WARNING: 
document isn't included in any toctree
docs/Documentation/filesystems/ext4/super.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/fmc/index.rst: WARNING: document isn't included in 
any toctree
docs/Documentation/gpu/msm-crash-dump.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/interconnect/interconnect.rst: WARNING: document 
isn't included in any toctree
docs/Documentation/laptops/lg-laptop.rst: WARNING: document isn't 
included in any toctree
docs/Documentation/virtual/kvm/amd-memory-encryption.rst: WARNING: 
document isn't included in any toctree
docs/Documentation/virtual/kvm/vcpu-requests.rst: WARNING: document 
isn't included in any toctree

After thinking a little bit and doing some tests, I think a good solution
would be to add ":orphan:" markup to the new .rst files that were not
added yet into the main body (e. g. something like the enclosed example).

That will make Sphinx suppress the:
"WARNING: document isn't included in any toctree"
warning for the new files, while they're not included at the main indexes.

This way, maintainers can do all the hard work of doing/applying the ReST
file conversion/addition patch series on their own trees, and, once
everything gets merged, submit a patch against the latest docs-next
tree, removing the :orphan: tag and add them to the common index.rst
files.

That should largely avoid merging conflicts.

For example, assuming that someone converts the stuff at
Documentation/accounting and rename the text files there to
RST (I'm actually doing that), he could add the enclosed change on
the top of its index file:

diff --git a/Documentation/accounting/index.rst 
b/Documentation/accounting/index.rst
index e7dda5afa73f..e1f6284b5ff3 100644
--- a/Documentation/accounting/index.rst
+++ b/Documentation/accounting/index.rst
@@ -1,3 +1,5 @@
+:orphan:
+
 ==
 Accounting
 ==

With would make Sphinx to ignore the subdir index file while
reporting errors. It will still build the documentation, allowing
the developer to test the changes.

One of the advantages is that checking the orphaned docs is as
easy as running:

$ git grep -l ":orphan:" Documentation/
...
Documentation/accounting/index.rst
 

[PATCH v3 4/6] drm/modes: Parse overscan properties

2019-04-18 Thread Maxime Ripard
Properly configuring the overscan properties might be needed for the
initial setup of the framebuffer for display that still have overscan.
Let's allow for more properties on the kernel command line to setup each
margin.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_modes.c | 44 ++-
 include/drm/drm_connector.h | 14 -
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac8d70b92b62..d93c44a97ce9 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1586,6 +1586,50 @@ static int drm_mode_parse_cmdline_options(char *str, 
size_t len,
} else if (!strncmp(option, "reflect_y", delim - option)) {
rotation |= DRM_MODE_REFLECT_Y;
sep = delim;
+   } else if (!strncmp(option, "margin_right", delim - option)) {
+   const char *value = delim + 1;
+   unsigned int margin;
+
+   margin = simple_strtol(value, &sep, 10);
+
+   /* Make sure we have parsed something */
+   if (sep == value)
+   return -EINVAL;
+
+   mode->tv_margins.right = margin;
+   } else if (!strncmp(option, "margin_left", delim - option)) {
+   const char *value = delim + 1;
+   unsigned int margin;
+
+   margin = simple_strtol(value, &sep, 10);
+
+   /* Make sure we have parsed something */
+   if (sep == value)
+   return -EINVAL;
+
+   mode->tv_margins.left = margin;
+   } else if (!strncmp(option, "margin_top", delim - option)) {
+   const char *value = delim + 1;
+   unsigned int margin;
+
+   margin = simple_strtol(value, &sep, 10);
+
+   /* Make sure we have parsed something */
+   if (sep == value)
+   return -EINVAL;
+
+   mode->tv_margins.top = margin;
+   } else if (!strncmp(option, "margin_bottom", delim - option)) {
+   const char *value = delim + 1;
+   unsigned int margin;
+
+   margin = simple_strtol(value, &sep, 10);
+
+   /* Make sure we have parsed something */
+   if (sep == value)
+   return -EINVAL;
+
+   mode->tv_margins.bottom = margin;
} else {
return -EINVAL;
}
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 6f57c1a3afff..89bc6ac38043 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -917,6 +917,20 @@ struct drm_cmdline_mode {
 * DRM_MODE_ROTATE_180 are supported at the moment.
 */
unsigned int rotation;
+
+   /**
+* @tv_margins: TV margins (in pixels)
+* @tv_margins.left: left margin
+* @tv_margins.right: right margin
+* @tv_margins.top: top margin
+* @tv_margins.bottom: bottom margin
+*/
+   struct {
+   unsigned int left;
+   unsigned int right;
+   unsigned int top;
+   unsigned int bottom;
+   } tv_margins;
 };
 
 /**
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 3/6] drm/modes: Allow to specify rotation and reflection on the commandline

2019-04-18 Thread Maxime Ripard
Rotations and reflections setup are needed in some scenarios to initialise
properly the initial framebuffer. Some drivers already had a bunch of
quirks to deal with this, such as either a private kernel command line
parameter (omapdss) or on the device tree (various panels).

In order to accomodate this, let's create a video mode parameter to deal
with the rotation and reflexion.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_client_modeset.c |  10 +++-
 drivers/gpu/drm/drm_modes.c  | 110 ++--
 include/drm/drm_connector.h  |   9 ++-
 3 files changed, 109 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c 
b/drivers/gpu/drm/drm_client_modeset.c
index f2869c82510c..15145d2c86d5 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -823,6 +823,7 @@ EXPORT_SYMBOL(drm_client_modeset_probe);
 bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int 
*rotation)
 {
struct drm_connector *connector = modeset->connectors[0];
+   struct drm_cmdline_mode *cmdline;
struct drm_plane *plane = modeset->crtc->primary;
u64 valid_mask = 0;
unsigned int i;
@@ -844,6 +845,15 @@ bool drm_client_panel_rotation(struct drm_mode_set 
*modeset, unsigned int *rotat
*rotation = DRM_MODE_ROTATE_0;
}
 
+   /**
+* We want the rotation on the command line to overwrite
+* whatever comes from the panel.
+*/
+   cmdline = &connector->cmdline_mode;
+   if (cmdline->specified &&
+   cmdline->rotation != DRM_MODE_ROTATE_0)
+   *rotation = cmdline->rotation;
+
/*
 * TODO: support 90 / 270 degree hardware rotation,
 * depending on the hardware this may require the framebuffer
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 9613c1a28487..ac8d70b92b62 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1531,6 +1531,71 @@ static int drm_mode_parse_cmdline_res_mode(const char 
*str, unsigned int length,
return 0;
 }
 
+static int drm_mode_parse_cmdline_options(char *str, size_t len,
+ struct drm_connector *connector,
+ struct drm_cmdline_mode *mode)
+{
+   unsigned int rotation = 0;
+   char *sep = str;
+
+   while ((sep = strchr(sep, ','))) {
+   char *delim, *option;
+
+   option = sep + 1;
+   delim = strchr(option, '=');
+   if (!delim) {
+   delim = strchr(option, ',');
+
+   if (!delim)
+   delim = str + len;
+   }
+
+   if (!strncmp(option, "rotate", delim - option)) {
+   const char *value = delim + 1;
+   unsigned int deg;
+
+   deg = simple_strtol(value, &sep, 10);
+
+   /* Make sure we have parsed something */
+   if (sep == value)
+   return -EINVAL;
+
+   switch (deg) {
+   case 0:
+   rotation |= DRM_MODE_ROTATE_0;
+   break;
+
+   case 90:
+   rotation |= DRM_MODE_ROTATE_90;
+   break;
+
+   case 180:
+   rotation |= DRM_MODE_ROTATE_180;
+   break;
+
+   case 270:
+   rotation |= DRM_MODE_ROTATE_270;
+   break;
+
+   default:
+   return -EINVAL;
+   }
+   } else if (!strncmp(option, "reflect_x", delim - option)) {
+   rotation |= DRM_MODE_REFLECT_X;
+   sep = delim;
+   } else if (!strncmp(option, "reflect_y", delim - option)) {
+   rotation |= DRM_MODE_REFLECT_Y;
+   sep = delim;
+   } else {
+   return -EINVAL;
+   }
+   }
+
+   mode->rotation = rotation;
+
+   return 0;
+}
+
 /**
  * drm_mode_parse_command_line_for_connector - parse command line modeline for 
connector
  * @mode_option: optional per connector mode option
@@ -1558,9 +1623,10 @@ bool drm_mode_parse_command_line_for_connector(const 
char *mode_option,
 {
const char *name;
bool named_mode = false, parse_extras = false;
-   unsigned int bpp_off = 0, refresh_off = 0;
+   unsigned int bpp_off = 0, refresh_off = 0, options_off = 0;
unsigned int mode_end = 0;
char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
+   char *options_ptr = NULL;
char *bpp_end_ptr = NULL, *refresh_end_ptr = 

[PATCH v3 0/6] drm/vc4: Allow for more boot-time configuration

2019-04-18 Thread Maxime Ripard
Hi,

The proprietary stack for the RaspberryPi allows for a number of video
parameters widely used by their users, but yet don't have any equivalents
in the mainline kernel.

Those options are detailed here:
https://www.raspberrypi.org/documentation/configuration/config-txt/video.md

While not all of them are desirable to have in the mainline kernel, some of
them still have value, such as properties to initialise the overscan or
rotation parameters.

This series is an attempt to support those, and is based on a rewrite of
the command line parser I did a couple of years ago that never reached
upstream (due to a lack of time on my side). While this parser was
initially made to deal with named modes (in order to support TV modes), it
also allowed to extend it more easily, which is why it's resurrected.

Let me know what you think,
Maxime

Changes from v2:
  - Fixed some sparse warnings
  - Rebased on top of next and Noralf series
  - Moved the property initialisation to vc4 reset hook
  - Added documentation for the new drm_cmdline_mode
  - Renamed overscan to tv_margins to be consistent with the APIs

Changes from v1:
  - Dropped the patches to deal with EDID
  - Added the unit test as selftest
  - Rebased on next

Maxime Ripard (6):
  drm/modes: Rewrite the command line parser
  drm/modes: Support modes names on the command line
  drm/modes: Allow to specify rotation and reflection on the commandline
  drm/modes: Parse overscan properties
  drm/selftests: Add command line parser selftests
  drm/vc4: hdmi: Set default state margin at reset

 drivers/gpu/drm/drm_client_modeset.c|  14 +-
 drivers/gpu/drm/drm_connector.c |   3 +-
 drivers/gpu/drm/drm_modes.c | 441 +--
 drivers/gpu/drm/selftests/Makefile  |   2 +-
 drivers/gpu/drm/selftests/drm_cmdline_selftests.h   |  49 +-
 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 846 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c  |  16 +-
 include/drm/drm_connector.h |  24 +-
 8 files changed, 1277 insertions(+), 118 deletions(-)
 create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h
 create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c

base-commit: ea2c44e9e7b523296ff8e26943bfdd0758d60104
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 6/6] drm/vc4: hdmi: Set default state margin at reset

2019-04-18 Thread Maxime Ripard
Now that the TV margins are properly parsed and filled into
drm_cmdline_mode, we just need to initialise the first state at reset to
get those values and start using them.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 99fc8569e0f5..d86f154138f5 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -255,11 +255,25 @@ static int vc4_hdmi_connector_get_modes(struct 
drm_connector *connector)
return ret;
 }
 
+static void vc4_hdmi_connector_reset(struct drm_connector *connector)
+{
+   struct drm_cmdline_mode *cmdline = &connector->cmdline_mode;
+   struct drm_connector_state *state;
+
+   drm_atomic_helper_connector_reset(connector);
+
+   state = connector->state;
+   state->tv.margins.left = cmdline->tv_margins.left;
+   state->tv.margins.right = cmdline->tv_margins.right;
+   state->tv.margins.top = cmdline->tv_margins.top;
+   state->tv.margins.bottom = cmdline->tv_margins.bottom;
+}
+
 static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
.detect = vc4_hdmi_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = vc4_hdmi_connector_destroy,
-   .reset = drm_atomic_helper_connector_reset,
+   .reset = vc4_hdmi_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 1/6] drm/modes: Rewrite the command line parser

2019-04-18 Thread Maxime Ripard
From: Maxime Ripard 

Rewrite the command line parser in order to get away from the state machine
parsing the video mode lines.

Hopefully, this will allow to extend it more easily to support named modes
and / or properties set directly on the command line.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_modes.c | 305 +++--
 1 file changed, 190 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 56f92a0bba62..3f89198f0891 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -30,6 +30,7 @@
  * authorization from the copyright holder(s) and author(s).
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -1405,6 +1406,131 @@ void drm_connector_list_update(struct drm_connector 
*connector)
 }
 EXPORT_SYMBOL(drm_connector_list_update);
 
+static int drm_mode_parse_cmdline_bpp(const char *str, char **end_ptr,
+ struct drm_cmdline_mode *mode)
+{
+   if (str[0] != '-')
+   return -EINVAL;
+
+   mode->bpp = simple_strtol(str + 1, end_ptr, 10);
+   mode->bpp_specified = true;
+
+   return 0;
+}
+
+static int drm_mode_parse_cmdline_refresh(const char *str, char **end_ptr,
+ struct drm_cmdline_mode *mode)
+{
+   if (str[0] != '@')
+   return -EINVAL;
+
+   mode->refresh = simple_strtol(str + 1, end_ptr, 10);
+   mode->refresh_specified = true;
+
+   return 0;
+}
+
+static int drm_mode_parse_cmdline_extra(const char *str, int length,
+   struct drm_connector *connector,
+   struct drm_cmdline_mode *mode)
+{
+   int i;
+
+   for (i = 0; i < length; i++) {
+   switch (str[i]) {
+   case 'i':
+   mode->interlace = true;
+   break;
+   case 'm':
+   mode->margins = true;
+   break;
+   case 'D':
+   if (mode->force != DRM_FORCE_UNSPECIFIED)
+   return -EINVAL;
+
+   if ((connector->connector_type != 
DRM_MODE_CONNECTOR_DVII) &&
+   (connector->connector_type != 
DRM_MODE_CONNECTOR_HDMIB))
+   mode->force = DRM_FORCE_ON;
+   else
+   mode->force = DRM_FORCE_ON_DIGITAL;
+   break;
+   case 'd':
+   if (mode->force != DRM_FORCE_UNSPECIFIED)
+   return -EINVAL;
+
+   mode->force = DRM_FORCE_OFF;
+   break;
+   case 'e':
+   if (mode->force != DRM_FORCE_UNSPECIFIED)
+   return -EINVAL;
+
+   mode->force = DRM_FORCE_ON;
+   break;
+   default:
+   return -EINVAL;
+   }
+   }
+
+   return 0;
+}
+
+static int drm_mode_parse_cmdline_res_mode(const char *str, unsigned int 
length,
+  bool extras,
+  struct drm_connector *connector,
+  struct drm_cmdline_mode *mode)
+{
+   bool rb = false, cvt = false;
+   int xres = 0, yres = 0;
+   int remaining, i;
+   char *end_ptr;
+
+   xres = simple_strtol(str, &end_ptr, 10);
+
+   if (end_ptr[0] != 'x')
+   return -EINVAL;
+   end_ptr++;
+
+   yres = simple_strtol(end_ptr, &end_ptr, 10);
+
+   remaining = length - (end_ptr - str);
+   if (remaining < 0)
+   return -EINVAL;
+
+   for (i = 0; i < remaining; i++) {
+   switch (end_ptr[i]) {
+   case 'M':
+   cvt = true;
+   break;
+   case 'R':
+   rb = true;
+   break;
+   default:
+   /*
+* Try to pass that to our extras parsing
+* function to handle the case where the
+* extras are directly after the resolution
+*/
+   if (extras) {
+   int ret = drm_mode_parse_cmdline_extra(end_ptr 
+ i,
+  1,
+  
connector,
+  mode);
+   if (ret)
+   return ret;
+   } else {
+   return -EINVAL;
+   }
+   }
+   }
+
+   mode->xres = xres;
+   mode->

[PATCH v3 5/6] drm/selftests: Add command line parser selftests

2019-04-18 Thread Maxime Ripard
The command line parser is pretty tough to get right and very error prone,
so let's add a selftest to try to catch any regression.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/selftests/Makefile  |   2 +-
 drivers/gpu/drm/selftests/drm_cmdline_selftests.h   |  49 +-
 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c | 846 +-
 3 files changed, 896 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h
 create mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c

diff --git a/drivers/gpu/drm/selftests/Makefile 
b/drivers/gpu/drm/selftests/Makefile
index 1bb73dc4c88c..fdd4238cb34a 100644
--- a/drivers/gpu/drm/selftests/Makefile
+++ b/drivers/gpu/drm/selftests/Makefile
@@ -2,4 +2,4 @@ test-drm_modeset-y := test-drm_modeset_common.o 
test-drm_plane_helper.o \
   test-drm_format.o test-drm_framebuffer.o \
  test-drm_damage_helper.o
 
-obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o
+obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o 
test-drm_cmdline_parser.o
diff --git a/drivers/gpu/drm/selftests/drm_cmdline_selftests.h 
b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
new file mode 100644
index ..6f9ba9459bec
--- /dev/null
+++ b/drivers/gpu/drm/selftests/drm_cmdline_selftests.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* List each unit test as selftest(function)
+ *
+ * The name is used as both an enum and expanded as igt__name to create
+ * a module parameter. It must be unique and legal for a C identifier.
+ *
+ * Tests are executed in order by igt/drm_mm
+ */
+
+#define cmdline_test(test) selftest(test, test)
+
+cmdline_test(drm_cmdline_test_res)
+cmdline_test(drm_cmdline_test_res_vesa)
+cmdline_test(drm_cmdline_test_res_vesa_rblank)
+cmdline_test(drm_cmdline_test_res_rblank)
+cmdline_test(drm_cmdline_test_res_bpp)
+cmdline_test(drm_cmdline_test_res_refresh)
+cmdline_test(drm_cmdline_test_res_bpp_refresh)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_margins)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_off)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_off)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_analog)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_force_on_digital)
+cmdline_test(drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on)
+cmdline_test(drm_cmdline_test_res_margins_force_on)
+cmdline_test(drm_cmdline_test_res_vesa_margins)
+cmdline_test(drm_cmdline_test_res_invalid_mode)
+cmdline_test(drm_cmdline_test_res_bpp_wrong_place_mode)
+cmdline_test(drm_cmdline_test_name)
+cmdline_test(drm_cmdline_test_name_bpp)
+cmdline_test(drm_cmdline_test_name_refresh)
+cmdline_test(drm_cmdline_test_name_bpp_refresh)
+cmdline_test(drm_cmdline_test_name_refresh_wrong_mode)
+cmdline_test(drm_cmdline_test_name_refresh_invalid_mode)
+cmdline_test(drm_cmdline_test_name_option)
+cmdline_test(drm_cmdline_test_name_bpp_option)
+cmdline_test(drm_cmdline_test_rotate_0)
+cmdline_test(drm_cmdline_test_rotate_90)
+cmdline_test(drm_cmdline_test_rotate_180)
+cmdline_test(drm_cmdline_test_rotate_270)
+cmdline_test(drm_cmdline_test_rotate_invalid_val)
+cmdline_test(drm_cmdline_test_rotate_truncated)
+cmdline_test(drm_cmdline_test_hmirror)
+cmdline_test(drm_cmdline_test_vmirror)
+cmdline_test(drm_cmdline_test_margin_options)
+cmdline_test(drm_cmdline_test_multiple_options)
+cmdline_test(drm_cmdline_test_invalid_option)
diff --git a/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c 
b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
new file mode 100644
index ..290d32cd9a07
--- /dev/null
+++ b/drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
@@ -0,0 +1,846 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Bootlin
+ */
+
+#define pr_fmt(fmt) "drm_cmdline: " fmt
+
+#include 
+#include 
+
+#include 
+#include 
+
+#define TESTS "drm_cmdline_selftests.h"
+#include "drm_selftest.h"
+#include "test-drm_modeset_common.h"
+
+static int drm_cmdline_test_res(void *ignored)
+{
+   struct drm_connector connector = { };
+   struct drm_cmdline_mode mode = { };
+
+   FAIL_ON(!drm_mode_parse_command_line_for_connector("720x480",
+  &connector,
+  &mode));
+   FAIL_ON(!mode.specified);
+   FAIL_ON(mode.xres != 720);
+   FAIL_ON(mode.yres != 480);
+
+   FAIL_ON(mode.refresh_specified);
+
+   FAIL_ON(mode.bpp_specified);
+
+   FAIL_ON(mode.rb);
+   FAIL_ON(mode.cvt);
+   FAIL_ON(mode.interlace);
+   FAIL_ON(mode.margins);
+   FAIL_ON(mode.force != DRM_FORCE_UNSPECIFIED);
+
+   return 0;
+}
+
+static int drm_cmdline_test_res_vesa(void *ignored)
+{
+   struct drm_connector connector = { };
+ 

[PATCH v3 2/6] drm/modes: Support modes names on the command line

2019-04-18 Thread Maxime Ripard
From: Maxime Ripard 

The drm subsystem also uses the video= kernel parameter, and in the
documentation refers to the fbdev documentation for that parameter.

However, that documentation also says that instead of giving the mode using
its resolution we can also give a name. However, DRM doesn't handle that
case at the moment. Even though in most case it shouldn't make any
difference, it might be useful for analog modes, where different standards
might have the same resolution, but still have a few different parameters
that are not encoded in the modes (NTSC vs NTSC-J vs PAL-M for example).

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_client_modeset.c |  4 ++-
 drivers/gpu/drm/drm_connector.c  |  3 +-
 drivers/gpu/drm/drm_modes.c  | 52 -
 include/drm/drm_connector.h  |  1 +-
 4 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_client_modeset.c 
b/drivers/gpu/drm/drm_client_modeset.c
index 2a428ac00930..f2869c82510c 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -149,6 +149,10 @@ drm_connector_pick_cmdline_mode(struct drm_connector 
*connector)
prefer_non_interlace = !cmdline_mode->interlace;
 again:
list_for_each_entry(mode, &connector->modes, head) {
+   /* Check (optional) mode name first */
+   if (!strcmp(mode->name, cmdline_mode->name))
+   return mode;
+
/* check width/height */
if (mode->hdisplay != cmdline_mode->xres ||
mode->vdisplay != cmdline_mode->yres)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124849db..e33814f5940e 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -139,8 +139,9 @@ static void drm_connector_get_cmdline_mode(struct 
drm_connector *connector)
connector->force = mode->force;
}
 
-   DRM_DEBUG_KMS("cmdline mode for connector %s %dx%d@%dHz%s%s%s\n",
+   DRM_DEBUG_KMS("cmdline mode for connector %s %s %dx%d@%dHz%s%s%s\n",
  connector->name,
+ mode->name ? mode->name : "",
  mode->xres, mode->yres,
  mode->refresh_specified ? mode->refresh : 60,
  mode->rb ? " reduced blanking" : "",
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 3f89198f0891..9613c1a28487 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1557,7 +1557,7 @@ bool drm_mode_parse_command_line_for_connector(const char 
*mode_option,
   struct drm_cmdline_mode *mode)
 {
const char *name;
-   bool parse_extras = false;
+   bool named_mode = false, parse_extras = false;
unsigned int bpp_off = 0, refresh_off = 0;
unsigned int mode_end = 0;
char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL;
@@ -1576,8 +1576,14 @@ bool drm_mode_parse_command_line_for_connector(const 
char *mode_option,
 
name = mode_option;
 
+   /*
+* If the first character is not a digit, then it means that
+* we have a named mode.
+*/
if (!isdigit(name[0]))
-   return false;
+   named_mode = true;
+   else
+   named_mode = false;
 
/* Try to locate the bpp and refresh specifiers, if any */
bpp_ptr = strchr(name, '-');
@@ -1588,6 +1594,9 @@ bool drm_mode_parse_command_line_for_connector(const char 
*mode_option,
 
refresh_ptr = strchr(name, '@');
if (refresh_ptr) {
+   if (named_mode)
+   return false;
+
refresh_off = refresh_ptr - name;
mode->refresh_specified = true;
}
@@ -1604,12 +1613,16 @@ bool drm_mode_parse_command_line_for_connector(const 
char *mode_option,
parse_extras = true;
}
 
-   ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
- parse_extras,
- connector,
- mode);
-   if (ret)
-   return false;
+   if (named_mode) {
+   strncpy(mode->name, name, mode_end);
+   } else {
+   ret = drm_mode_parse_cmdline_res_mode(name, mode_end,
+ parse_extras,
+ connector,
+ mode);
+   if (ret)
+   return false;
+   }
mode->specified = true;
 
if (bpp_ptr) {
@@ -1637,14 +1650,23 @@ bool drm_mode_parse_command_line_for_connector(const 
char *mode_option,
extra_ptr = refresh_end_ptr;
 
if (extra_ptr) {
-   int remaining = 

Re: [PATCH 00/20] drm: Split out the formats API and move it to a common place

2019-04-18 Thread Daniel Vetter
On Thu, Apr 18, 2019 at 2:01 PM Maxime Ripard  wrote:
> On Thu, Apr 18, 2019 at 12:07:44PM +0200, Daniel Vetter wrote:
> > On Thu, Apr 18, 2019 at 11:02 AM Maxime Ripard
> >  wrote:
> > >
> > > On Thu, Apr 18, 2019 at 09:52:10AM +0200, Daniel Vetter wrote:
> > > > On Thu, Apr 18, 2019 at 8:22 AM Maxime Ripard 
> > > >  wrote:
> > > > > On Wed, Apr 17, 2019 at 05:41:21PM +0200, Daniel Vetter wrote:
> > > > > > On Wed, Apr 17, 2019 at 09:54:26AM +0200, Maxime Ripard wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > DRM comes with an extensive format support to retrieve the various
> > > > > > > parameters associated with a given format (such as the 
> > > > > > > subsampling, or the
> > > > > > > bits per pixel), as well as some helpers and utilities to ease 
> > > > > > > the driver
> > > > > > > development.
> > > > > > >
> > > > > > > v4l2, on the other side, doesn't provide such facilities, leaving 
> > > > > > > each
> > > > > > > driver reimplement a subset of the formats parameters for the one 
> > > > > > > supported
> > > > > > > by that particular driver. This leads to a lot of duplication and
> > > > > > > boilerplate code in the v4l2 drivers.
> > > > > > >
> > > > > > > This series tries to address this by moving the DRM format API 
> > > > > > > into lib and
> > > > > > > turning it into a more generic API. In order to do this, we've 
> > > > > > > needed to do
> > > > > > > some preliminary changes on the DRM drivers, then moved the API 
> > > > > > > and finally
> > > > > > > converted a v4l2 driver to give an example of how such library 
> > > > > > > could be
> > > > > > > used.
> > > > > > >
> > > > > > > Let me know what you think,
> > > > > > > Maxime
> > > > > > >
> > > > > > > Changes from RFC:
> > > > > > >   - Rebased on next
> > > > > > >   - Fixed the various formats mapping
> > > > > > >   - Added tags
> > > > > > >   - Did most of the formats functions as inline functions
> > > > > > >   - Used a consistent prefix for all the utilities functions
> > > > > > >   - Fixed the compilation breakages, and did a run of 
> > > > > > > allmodconfig for arm,
> > > > > > > arm64 and x86_64
> > > > > > >   - Fixed out of array bounds warnings in the 
> > > > > > > image_format_info_block_*
> > > > > > > functions
> > > > > > >   - Added License and copyright headers on missing files
> > > > > > >
> > > > > > > Maxime Ripard (20):
> > > > > > >   drm: Remove users of drm_format_num_planes
> > > > > > >   drm: Remove users of drm_format_(horz|vert)_chroma_subsampling
> > > > > > >   drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
> > > > > > >   drm/fourcc: Pass the format_info pointer to 
> > > > > > > drm_format_plane_width/height
> > > > > > >   drm: Replace instances of drm_format_info by drm_get_format_info
> > > > > > >   lib: Add video format information library
> > > > > > >   drm/fb: Move from drm_format_info to image_format_info
> > > > > > >   drm/malidp: Convert to generic image format library
> > > > > > >   drm/client: Convert to generic image format library
> > > > > > >   drm/exynos: Convert to generic image format library
> > > > > > >   drm/i915: Convert to generic image format library
> > > > > > >   drm/ipuv3: Convert to generic image format library
> > > > > > >   drm/msm: Convert to generic image format library
> > > > > > >   drm/omap: Convert to generic image format library
> > > > > > >   drm/rockchip: Convert to generic image format library
> > > > > > >   drm/tegra: Convert to generic image format library
> > > > > > >   drm/fourcc: Remove old DRM format API
> > > > > > >   lib: image-formats: Add v4l2 formats support
> > > > > > >   lib: image-formats: Add more functions
> > > > > > >   media: sun6i: Convert to the image format API
> > > > > >
> > > > > > In the interest of making myself unpopular: Why move this out of 
> > > > > > drm?
> > > > > >
> > > > > > We do have a bunch of other such shared helpers already (mostly in
> > > > > > drivers/video) for dt videomode and hdmi infoframes, and I'm not 
> > > > > > super
> > > > > > sure that's going better than keeping it maintained in drm.
> > > > > >
> > > > > > Plus the uapi is already that you include drm_fourcc.h to get at 
> > > > > > these,
> > > > > > dropping the drm prefix isn't going to help I think.
> > > > > >
> > > > > > Of course we'd need to make it a separate drm_formats.ko (so that 
> > > > > > v4l can
> > > > > > use it without dragging in all of drm), and we need to add some 
> > > > > > fields for
> > > > > > converting to v4l fourcc codes (which are different). But that 
> > > > > > should be
> > > > > > all possible. And I don't think the drm_ prefix in v4l code is a 
> > > > > > problem,
> > > > > > it's actually a feature: It makes it really clear that these are 
> > > > > > the drm
> > > > > > fourcc codes, as allocated in drm_fourcc.h, plus their modifiers, 
> > > > > > and all
> > > > > > that. That allocation authority is also already baked into various 
> > > 

DMA-buf P2P

2019-04-18 Thread Christian König
Hi guys,

as promised this is the patch set which enables P2P buffer sharing with DMA-buf.

Basic idea is that importers can set a flag noting that they can deal with and 
sgt which doesn't contains pages.

This in turn is the signal to the exporter that we don't need to move a buffer 
to system memory any more when a remote device wants to access it.

Please review and/or comment,
Christian.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/amdgpu: add support for exporting VRAM using DMA-buf v2

2019-04-18 Thread Christian König
We should be able to do this now after checking all the prerequisites.

v2: fix entrie count in the sgt

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c| 46 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h  |  9 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 96 
 3 files changed, 142 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index a290ae830b11..55bb39281c5d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -318,22 +318,45 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
}
 
if (attach->invalidate) {
-   /* move buffer into GTT */
+   /* move buffer into GTT or VRAM */
struct ttm_operation_ctx ctx = { false, false };
+   unsigned domains = AMDGPU_GEM_DOMAIN_GTT;
 
-   amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
+   if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
+   attach->peer2peer) {
+   bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+   domains |= AMDGPU_GEM_DOMAIN_VRAM;
+   }
+   amdgpu_bo_placement_from_domain(bo, domains);
r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
if (r)
return ERR_PTR(r);
}
 
-   sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, bo->tbo.num_pages);
-   if (IS_ERR(sgt))
-   return sgt;
+   switch (bo->tbo.mem.mem_type) {
+   case TTM_PL_TT:
+   sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
+   bo->tbo.num_pages);
+   if (IS_ERR(sgt))
+   return sgt;
+
+   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
+ DMA_ATTR_SKIP_CPU_SYNC)) {
+   r = -EINVAL;
+   goto error_free;
+   }
+   break;
 
-   if (!dma_map_sg_attrs(attach->dev, sgt->sgl, sgt->nents, dir,
- DMA_ATTR_SKIP_CPU_SYNC))
+   case TTM_PL_VRAM:
+   r = amdgpu_vram_mgr_alloc_sgt(adev, &bo->tbo.mem, attach->dev,
+ dir, &sgt);
+   if (r)
+   goto error_free;
+   break;
+   default:
+   r = -EINVAL;
goto error_free;
+   }
 
if (attach->dev->driver != adev->dev->driver)
bo->prime_shared_count++;
@@ -343,7 +366,7 @@ amdgpu_gem_map_dma_buf(struct dma_buf_attachment *attach,
 error_free:
sg_free_table(sgt);
kfree(sgt);
-   return ERR_PTR(-ENOMEM);
+   return ERR_PTR(r);
 }
 
 /**
@@ -367,10 +390,15 @@ static void amdgpu_gem_unmap_dma_buf(struct 
dma_buf_attachment *attach,
if (attach->dev->driver != adev->dev->driver && bo->prime_shared_count)
bo->prime_shared_count--;
 
-   if (sgt) {
+   if (!sgt)
+   return;
+
+   if (sgt->sgl->page_link) {
dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
sg_free_table(sgt);
kfree(sgt);
+   } else {
+   amdgpu_vram_mgr_free_sgt(adev, attach->dev, dir, sgt);
}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index c2b7669004ba..0b4cdbe867e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -72,6 +72,15 @@ uint64_t amdgpu_gtt_mgr_usage(struct ttm_mem_type_manager 
*man);
 int amdgpu_gtt_mgr_recover(struct ttm_mem_type_manager *man);
 
 u64 amdgpu_vram_mgr_bo_visible_size(struct amdgpu_bo *bo);
+int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
+ struct ttm_mem_reg *mem,
+ struct device *dev,
+ enum dma_data_direction dir,
+ struct sg_table **sgt);
+void amdgpu_vram_mgr_free_sgt(struct amdgpu_device *adev,
+ struct device *dev,
+ enum dma_data_direction dir,
+ struct sg_table *sgt);
 uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
 uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index ec9ea3fdbb4a..520cea4dbdab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -399,6 +399,102 @@ static void amdgpu_vram_mgr_del(struct 
ttm_mem_type_manager *man,
mem->mm_node = NULL;
 }
 
+/**
+ * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg

[PATCH 2/6] PCI/P2PDMA: start with a whitelist for root complexes

2019-04-18 Thread Christian König
A lot of root complexes can still do P2P even when PCI devices
don't share a common upstream bridge.

Start adding a whitelist and allow P2P if both participants are
attached to known good root complex.

Signed-off-by: Christian König 
---
 drivers/pci/p2pdma.c | 38 +++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index c52298d76e64..212baaa7f93b 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -274,6 +274,31 @@ static void seq_buf_print_bus_devfn(struct seq_buf *buf, 
struct pci_dev *pdev)
seq_buf_printf(buf, "%s;", pci_name(pdev));
 }
 
+/*
+ * If we can't find a common upstream bridge take a look at the root complex 
and
+ * compare it to a whitelist of known good hardware.
+ */
+static bool root_complex_whitelist(struct pci_dev *dev)
+{
+   struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
+   struct pci_dev *root = pci_get_slot(host->bus, PCI_DEVFN(0, 0));
+   unsigned short vendor, device;
+
+   if (!root)
+   return false;
+
+   vendor = root->vendor;
+   device = root->device;
+   pci_dev_put(root);
+
+   /* AMD ZEN host bridges can do peer to peer */
+   if (vendor == PCI_VENDOR_ID_AMD && device == 0x1450)
+   return true;
+
+   /* TODO: Extend that to a proper whitelist */
+   return false;
+}
+
 /*
  * Find the distance through the nearest common upstream bridge between
  * two PCI devices.
@@ -317,13 +342,13 @@ static void seq_buf_print_bus_devfn(struct seq_buf *buf, 
struct pci_dev *pdev)
  * In this case, a list of all infringing bridge addresses will be
  * populated in acs_list (assuming it's non-null) for printk purposes.
  */
-static int upstream_bridge_distance(struct pci_dev *a,
-   struct pci_dev *b,
+static int upstream_bridge_distance(struct pci_dev *provider,
+   struct pci_dev *client,
struct seq_buf *acs_list)
 {
+   struct pci_dev *a = provider, *b = client, *bb;
int dist_a = 0;
int dist_b = 0;
-   struct pci_dev *bb = NULL;
int acs_cnt = 0;
 
/*
@@ -354,6 +379,13 @@ static int upstream_bridge_distance(struct pci_dev *a,
dist_a++;
}
 
+   /* Allow the connection if both devices are on a whitelisted root
+* complex, but add an arbitary large value to the distance.
+*/
+   if (root_complex_whitelist(provider) &&
+   root_complex_whitelist(client))
+   return 0x1000 + dist_a + dist_b;
+
return -1;
 
 check_b_path_acs:
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] lib/scatterlist: add sg_set_dma_addr() helper

2019-04-18 Thread Christian König
Use this function to set an sg entry to point to device resources mapped
using dma_map_resource(). The page pointer is set to NULL and only the DMA
address, length and offset values are valid.

Signed-off-by: Christian König 
---
 include/linux/scatterlist.h | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index b96f0d0b5b8f..d72f76768767 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -145,6 +145,29 @@ static inline void sg_set_buf(struct scatterlist *sg, 
const void *buf,
sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf));
 }
 
+/**
+ * sg_set_dma_addr - Set sg entry to point at specified dma address
+ * @sg: SG entry
+ * @address:DMA address to set
+ * @len:Length of data
+ * @offset: Offset into page
+ *
+ * Description:
+ *   Use this function to set an sg entry to point to device resources mapped
+ *   using dma_map_resource(). The page pointer is set to NULL and only the DMA
+ *   address, length and offset values are valid.
+ *
+ **/
+static inline void sg_set_dma_addr(struct scatterlist *sg, dma_addr_t address,
+  unsigned int len, unsigned int offset)
+{
+   sg_set_page(sg, NULL, len, offset);
+   sg->dma_address = address;
+#ifdef CONFIG_NEED_SG_DMA_LENGTH
+   sg->dma_length = len;
+#endif
+}
+
 /*
  * Loop over each sg element, following the pointer to a new list if necessary
  */
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/amdgpu: add checks if DMA-buf P2P is supported

2019-04-18 Thread Christian König
Check if we can do peer2peer on the PCIe bus.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index af103b7e21e8..a290ae830b11 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * amdgpu_gem_prime_vmap - &dma_buf_ops.vmap implementation
@@ -254,13 +255,27 @@ static int amdgpu_gem_dma_buf_attach(struct dma_buf 
*dma_buf,
 {
struct drm_gem_object *obj = dma_buf->priv;
struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+   if (!attach->peer2peer)
+   goto no_peer2peer;
+
+   if (!dev_is_pci(attach->dev))
+   goto no_peer2peer;
+
+   if (pci_p2pdma_distance_many(adev->pdev, &attach->dev, 1, true) < 0)
+   goto no_peer2peer;
+
+   return 0;
+
+no_peer2peer:
+   attach->peer2peer = false;
 
/* Make sure the buffer is pinned when userspace didn't set GTT as
 * preferred domain. This avoid ping/pong situations with scan out BOs.
 */
if (!(bo->preferred_domains & AMDGPU_GEM_DOMAIN_GTT))
attach->invalidate = NULL;
-
return 0;
 }
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] dma-buf: add peer2peer flag

2019-04-18 Thread Christian König
Add a peer2peer flag noting that the importer can deal with device
resources which are not backed by pages.

Signed-off-by: Christian König 
---
 drivers/dma-buf/dma-buf.c | 1 +
 include/linux/dma-buf.h   | 4 
 2 files changed, 5 insertions(+)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f23ff8355505..3ab49b492017 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -563,6 +563,7 @@ struct dma_buf_attachment *dma_buf_attach(const struct 
dma_buf_attach_info *info
 
attach->dev = info->dev;
attach->dmabuf = dmabuf;
+   attach->peer2peer = info->peer2peer;
attach->importer_priv = info->importer_priv;
attach->invalidate = info->invalidate;
 
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index a615b74e5894..d2746bdfe92c 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -346,6 +346,7 @@ struct dma_buf {
  * @dmabuf: buffer for this attachment.
  * @dev: device attached to the buffer.
  * @node: list of dma_buf_attachment.
+ * @peer2peer: true if the importer can handle peer resources without pages.
  * @priv: exporter specific attachment data.
  * @importer_priv: importer specific attachment data.
  *
@@ -362,6 +363,7 @@ struct dma_buf_attachment {
struct dma_buf *dmabuf;
struct device *dev;
struct list_head node;
+   bool peer2peer;
void *priv;
struct sg_table *sgt;
void *importer_priv;
@@ -427,6 +429,7 @@ struct dma_buf_export_info {
  * struct dma_buf_attach_info - holds information needed to attach to a dma_buf
  * @dmabuf:the exported dma_buf
  * @dev:   the device which wants to import the attachment
+ * @peer2peer: true if the importer can handle peer resources without 
pages
  * @importer_priv: private data of importer to this attachment
  * @invalidate:callback to use for invalidating mappings
  *
@@ -436,6 +439,7 @@ struct dma_buf_export_info {
 struct dma_buf_attach_info {
struct dma_buf *dmabuf;
struct device *dev;
+   bool peer2peer;
void *importer_priv;
void (*invalidate)(struct dma_buf_attachment *attach);
 };
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/amdgpu: note that we can handle peer2peer DMA-buf

2019-04-18 Thread Christian König
Importing should work out of the box.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
index 30634396719b..af103b7e21e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
@@ -519,6 +519,7 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
drm_device *dev,
if (IS_ERR(obj))
return obj;
 
+   attach_info.peer2peer = true;
attach_info.importer_priv = obj;
attach = dma_buf_attach(&attach_info);
if (IS_ERR(attach)) {
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/20] drm: Split out the formats API and move it to a common place

2019-04-18 Thread Maxime Ripard
On Thu, Apr 18, 2019 at 12:07:44PM +0200, Daniel Vetter wrote:
> On Thu, Apr 18, 2019 at 11:02 AM Maxime Ripard
>  wrote:
> >
> > On Thu, Apr 18, 2019 at 09:52:10AM +0200, Daniel Vetter wrote:
> > > On Thu, Apr 18, 2019 at 8:22 AM Maxime Ripard  
> > > wrote:
> > > > On Wed, Apr 17, 2019 at 05:41:21PM +0200, Daniel Vetter wrote:
> > > > > On Wed, Apr 17, 2019 at 09:54:26AM +0200, Maxime Ripard wrote:
> > > > > > Hi,
> > > > > >
> > > > > > DRM comes with an extensive format support to retrieve the various
> > > > > > parameters associated with a given format (such as the subsampling, 
> > > > > > or the
> > > > > > bits per pixel), as well as some helpers and utilities to ease the 
> > > > > > driver
> > > > > > development.
> > > > > >
> > > > > > v4l2, on the other side, doesn't provide such facilities, leaving 
> > > > > > each
> > > > > > driver reimplement a subset of the formats parameters for the one 
> > > > > > supported
> > > > > > by that particular driver. This leads to a lot of duplication and
> > > > > > boilerplate code in the v4l2 drivers.
> > > > > >
> > > > > > This series tries to address this by moving the DRM format API into 
> > > > > > lib and
> > > > > > turning it into a more generic API. In order to do this, we've 
> > > > > > needed to do
> > > > > > some preliminary changes on the DRM drivers, then moved the API and 
> > > > > > finally
> > > > > > converted a v4l2 driver to give an example of how such library 
> > > > > > could be
> > > > > > used.
> > > > > >
> > > > > > Let me know what you think,
> > > > > > Maxime
> > > > > >
> > > > > > Changes from RFC:
> > > > > >   - Rebased on next
> > > > > >   - Fixed the various formats mapping
> > > > > >   - Added tags
> > > > > >   - Did most of the formats functions as inline functions
> > > > > >   - Used a consistent prefix for all the utilities functions
> > > > > >   - Fixed the compilation breakages, and did a run of allmodconfig 
> > > > > > for arm,
> > > > > > arm64 and x86_64
> > > > > >   - Fixed out of array bounds warnings in the 
> > > > > > image_format_info_block_*
> > > > > > functions
> > > > > >   - Added License and copyright headers on missing files
> > > > > >
> > > > > > Maxime Ripard (20):
> > > > > >   drm: Remove users of drm_format_num_planes
> > > > > >   drm: Remove users of drm_format_(horz|vert)_chroma_subsampling
> > > > > >   drm/fourcc: Pass the format_info pointer to drm_format_plane_cpp
> > > > > >   drm/fourcc: Pass the format_info pointer to 
> > > > > > drm_format_plane_width/height
> > > > > >   drm: Replace instances of drm_format_info by drm_get_format_info
> > > > > >   lib: Add video format information library
> > > > > >   drm/fb: Move from drm_format_info to image_format_info
> > > > > >   drm/malidp: Convert to generic image format library
> > > > > >   drm/client: Convert to generic image format library
> > > > > >   drm/exynos: Convert to generic image format library
> > > > > >   drm/i915: Convert to generic image format library
> > > > > >   drm/ipuv3: Convert to generic image format library
> > > > > >   drm/msm: Convert to generic image format library
> > > > > >   drm/omap: Convert to generic image format library
> > > > > >   drm/rockchip: Convert to generic image format library
> > > > > >   drm/tegra: Convert to generic image format library
> > > > > >   drm/fourcc: Remove old DRM format API
> > > > > >   lib: image-formats: Add v4l2 formats support
> > > > > >   lib: image-formats: Add more functions
> > > > > >   media: sun6i: Convert to the image format API
> > > > >
> > > > > In the interest of making myself unpopular: Why move this out of drm?
> > > > >
> > > > > We do have a bunch of other such shared helpers already (mostly in
> > > > > drivers/video) for dt videomode and hdmi infoframes, and I'm not super
> > > > > sure that's going better than keeping it maintained in drm.
> > > > >
> > > > > Plus the uapi is already that you include drm_fourcc.h to get at 
> > > > > these,
> > > > > dropping the drm prefix isn't going to help I think.
> > > > >
> > > > > Of course we'd need to make it a separate drm_formats.ko (so that v4l 
> > > > > can
> > > > > use it without dragging in all of drm), and we need to add some 
> > > > > fields for
> > > > > converting to v4l fourcc codes (which are different). But that should 
> > > > > be
> > > > > all possible. And I don't think the drm_ prefix in v4l code is a 
> > > > > problem,
> > > > > it's actually a feature: It makes it really clear that these are the 
> > > > > drm
> > > > > fourcc codes, as allocated in drm_fourcc.h, plus their modifiers, and 
> > > > > all
> > > > > that. That allocation authority is also already baked into various 
> > > > > khr/ext
> > > > > standards, too.
> > > >
> > > > The way I see it, there's a fundamental difference between the UAPI
> > > > and the kernel. I don't suggest we change anything about the UAPI: the
> > > > drm formats should keep their prefix, drm_fourcc.h can remain that

Re: [PATCH 00/20] drm: Split out the formats API and move it to a common place

2019-04-18 Thread Paul Kocialkowski
Hi,

On Thu, 2019-04-18 at 11:02 +0200, Maxime Ripard wrote:
> On Thu, Apr 18, 2019 at 09:52:10AM +0200, Daniel Vetter wrote:
> > And a lot of people pushed for the "fourcc is a standard", when
> > really it's totally not.
> 
> Even if it's not a standard, having consistency would be a good thing.
> 
> And you said yourself that DRM fourcc is now pretty much an authority
> for the fourcc, so it definitely looks like a standard to me.

I think trying to make the V4L2 and DRM fourccs converge is a lost
cause, as it has already significantly diverged. Even if we coordinate
an effort to introduce new formats with the same fourcc on both sides,
I don't see what good that would be since the formats we have now are
still plagued by the inconsistency.

I think we always need an explicit translation step from either v4l2 or
drm to the internal representation and back, without ever assuming that
formats might be compatible because they share the same fourcc.

It looks like so far, V4L2 pixel formats describe a DRM pixel format +
modifier. I think Boris (CCed) is working to change that by allowing to
pass a DRM modifier through V4L2. With that, we'd be in a situation
where some formats are described by the v4l2 pixfmt alone and some
formats are also described a modifier (but I looked at it from a
distance so might have misunderstod). That feels better since it avoids
the combinatory explosion from describing each format + modifier
individually.

What do you think?

Cheers,

Paul

> > v4l tends to conflate pixel format with stuff that we tend to encode
> > in modifiers a lot more.
> 
> Boris is working on adding the modifiers concept to v4l2, so we're
> converging here, and we can totally have a layer in v4l2 to convert
> between old v4l2 "format+modifiers" formats, and DRM style formats.
> 
> > There's a bunch of reasons we can't just use v4l, and they're as
> > valid as ever:
> > 
> > - We overlap badly in some areas, so even if fourcc codes match, we
> >   can't use them and need a duplicated DRM_FOURCC code.
> 
> Do yo have an example of one of those areas?
> 
> > - v4l encodes some metadata into the fourcc that we encode elsewhere,
> >   e.g. offset for planar yuv formats, or tiling mode
> 
> As I was saying, this changes on the v4l2 side, and converging to
> what DRM is doing.
> 
> > - drm fourcc code doesn't actually define the drm_format_info
> >   uniquely, drivers can override that (that's an explicit design
> >   intent of modifiers, to allow drivers to add another plane for
> >   e.g. compression information). You'd need to pull that driver
> >   knowledge into your format library.
> 
> I'm not sure how my patches are changing anything here. This is
> litterally the same code, with the functions renamed.
> 
> If drivers want to override that, then yeah, fine, we can let them do
> that. Just like any helper this just provides a default that covers
> most of the cases.
> 
> > Iow there's no way we can easily adopt v4l fourcc, except if we do
> > something like a new addfb flag.
> 
> For the formats that would be described as a modifier, sure. For all
> the others (that are not yet supported by DRM), then I don't really
> see why not.
> 
> > > And given how the current state is a mess in this regard, I'm not too
> > > optimistic about keeping the formats in their relevant frameworks.
> > > 
> > > Having a shared library, governed by both, will make this far easier,
> > > since it will be easy to discover the formats that are already
> > > supported by the other subsystem.
> > 
> > I think a compat library that (tries to, best effort) convert between
> > v4l and drm fourcc would make sense. Somewhere in drivers/video, next
> > to the conversion functions for videomode <-> drm_display_mode
> > perhaps. That should be useful for drivers.
> 
> That's not really what this series is about though. That series is
> about sharing the (image|pixels) formats database across everyone so
> that everyone can benefit from it.
> 
> > Unifying the formats themselves, and all the associated metadata is
> > imo a no-go, and was a pretty conscious decision when we implemented
> > drm_fourcc a few years back.
> > 
> > > If we want to keep the current library in DRM, we have two options
> > > then:
> > > 
> > >   - Support all the v4l2 formats in the DRM library, which is
> > > essentially what I'm doing in the last patches. However, that
> > > would require to have the v4l2 developpers also reviewing stuff
> > > there. And given how busy they are, I cannot really see how that
> > > would work.
> > 
> > Well, if we end up with a common library then yes we need cross
> > review. There's no way around that. Doesn't matter where exactly that
> > library is in the filesystem tree, and adding a special MAINTAINERS
> > entry for anything related to fourcc (both drm and v4l) to make sure
> > they get cross-posted is easy. No file renaming needed.
> 
> That would create some governing issues as well. For example, if you
> 

  1   2   >