Re: [PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-14 Thread Brian Starkey 

On Tue, Jun 13, 2017 at 10:32:23AM -0700, Eric Anholt wrote:

Brian Starkey  writes:


Hi Boris,

Sorry lost track of this thread.


On Tue, Jun 06, 2017 at 09:13:00PM +0200, Boris Brezillon wrote:

Hi Brian,

Le Mon, 5 Jun 2017 12:25:50 +0100,
Brian Starkey  a écrit :


Hi Boris,

I can't speak for the HW-specific details, but the writeback part
looks pretty good (and familiar ;-) to me. There certainly seems to be
some scope for code-sharing there, but I think it's fine not to do it
yet.


Agreed.



I've a further query below about the handling of CRTC events.



[...]


>+
>+void vc4_txp_atomic_commit(struct drm_device *dev,
>+  struct drm_atomic_state *old_state)
>+{
>+   struct vc4_dev *vc4 = to_vc4_dev(dev);
>+   struct vc4_txp *txp = vc4->txp;
>+   struct drm_connector_state *conn_state = txp->connector.base.state;
>+   struct drm_display_mode *mode;
>+   struct drm_gem_cma_object *gem;
>+   struct drm_framebuffer *fb;
>+   u32 ctrl = TXP_GO | TXP_EI;
>+
>+   /* Writeback connector is disabled, nothing to do. */
>+   if (!conn_state->crtc)
>+   return;
>+
>+   /* Writeback connector is enabled, but has no FB assigned to it. Fake a
>+* vblank event to complete ->flip_done.
>+*/
>+   if (!conn_state->writeback_job || !conn_state->writeback_job->fb) {
>+   vc4_crtc_eof_event(conn_state->crtc);

I'm not sure about hiding away the one-shot thing like this. If the
CRTC remains "active" from the API point of view, I'd expect it to be
able to keep generating VBLANK events.

I don't know how to do if, but if there were some notion of
"auto-disabling" CRTCs then this quirk would go away, and you'd also
be able to enforce that the CRTC can't be enabled without a writeback
framebuffer.


I don't have a strong opinion on "auto-disabling CRTC" vs "fake VBLANK
events". Note that I'm already faking a VBLANK event when activating
writeback, because there's no such concept at the HVS/TXP level. We
do have EOF (End Of Frame) and writeback done events, but these are
quite independent from the VBLANK events generated by the pixelvalve
block (the block responsible for generating the HSYNC/VSYNC signals).



I'm also not sure how (if?) this works today with a CRTC driving a DSI
command-mode panel. Does the CRTC keep generating VBLANKs even when
there are no updates?


I don't know. Is this mandatory to have VBLANK events? I mean, the
core is using VBLANK events to detect when page flips have been done,
that is, when old framebuffers are unused and new ones started to be
fetched by the CRTC, but on some HW, VBLANK is not the only way to
detect such situations. The question is, are there other situations
where VBLANK events are required, or is there an implicit rule stating
that a CRTC has to generate VBLANK events at a vrefresh rate even when
the display is actually not updated when nothing changes?


I'm not sure how widely relied upon it is, but I'd expect that there's
a requirement to make sure DRM_IOCTL_WAIT_VBLANK works. I _think_ that
means the CRTC should generate events at vrefresh rate if there's a
wait request outstanding that depends on that.


In our case, there's no vrefresh rate, though.  Just completion.

I've been trying to come up with a usecase for vblank events on
writeack, and the best I have is: to enable VNC-style capture of a
complete hardware rendering stack, we could run modesetting against the
writeback connector and do one-shot writebacks when damage comes in.
You would want GL apps to be throttled to the frame capture rate, so X
needs to implement waits for at least a swapinterval of 1 (I don't see
how greater than 1 would make any sense)

However, given that you'd be triggering writebacks on damage, and using
the fence to trigger the next wait for damage and writeback already, I
think you'd use that set of code for Present/DRI2 swapinterval support,
not the current vblank path.

My current inclination would be to throw -EINVAL for userspace vblank
requests on writeback conncetor pipes.


I'm not sure how you'd plumb that in, but the behaviour sounds OK to
me. We can write-back at the same time as scanout to the display from
the same CRTC, so we'd not want to return -EINVAL in that case.

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

Re: [PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-13 Thread Eric Anholt 
Brian Starkey  writes:

> Hi Boris,
>
> Sorry lost track of this thread.
>
>
> On Tue, Jun 06, 2017 at 09:13:00PM +0200, Boris Brezillon wrote:
>>Hi Brian,
>>
>>Le Mon, 5 Jun 2017 12:25:50 +0100,
>>Brian Starkey  a écrit :
>>
>>> Hi Boris,
>>>
>>> I can't speak for the HW-specific details, but the writeback part
>>> looks pretty good (and familiar ;-) to me. There certainly seems to be
>>> some scope for code-sharing there, but I think it's fine not to do it
>>> yet.
>>
>>Agreed.
>>
>>>
>>> I've a further query below about the handling of CRTC events.
>>>
>>
>>[...]
>>
>>> >+
>>> >+void vc4_txp_atomic_commit(struct drm_device *dev,
>>> >+ struct drm_atomic_state *old_state)
>>> >+{
>>> >+  struct vc4_dev *vc4 = to_vc4_dev(dev);
>>> >+  struct vc4_txp *txp = vc4->txp;
>>> >+  struct drm_connector_state *conn_state = txp->connector.base.state;
>>> >+  struct drm_display_mode *mode;
>>> >+  struct drm_gem_cma_object *gem;
>>> >+  struct drm_framebuffer *fb;
>>> >+  u32 ctrl = TXP_GO | TXP_EI;
>>> >+
>>> >+  /* Writeback connector is disabled, nothing to do. */
>>> >+  if (!conn_state->crtc)
>>> >+  return;
>>> >+
>>> >+  /* Writeback connector is enabled, but has no FB assigned to it. Fake a
>>> >+   * vblank event to complete ->flip_done.
>>> >+   */
>>> >+  if (!conn_state->writeback_job || !conn_state->writeback_job->fb) {
>>> >+  vc4_crtc_eof_event(conn_state->crtc);
>>>
>>> I'm not sure about hiding away the one-shot thing like this. If the
>>> CRTC remains "active" from the API point of view, I'd expect it to be
>>> able to keep generating VBLANK events.
>>>
>>> I don't know how to do if, but if there were some notion of
>>> "auto-disabling" CRTCs then this quirk would go away, and you'd also
>>> be able to enforce that the CRTC can't be enabled without a writeback
>>> framebuffer.
>>
>>I don't have a strong opinion on "auto-disabling CRTC" vs "fake VBLANK
>>events". Note that I'm already faking a VBLANK event when activating
>>writeback, because there's no such concept at the HVS/TXP level. We
>>do have EOF (End Of Frame) and writeback done events, but these are
>>quite independent from the VBLANK events generated by the pixelvalve
>>block (the block responsible for generating the HSYNC/VSYNC signals).
>>
>>>
>>> I'm also not sure how (if?) this works today with a CRTC driving a DSI
>>> command-mode panel. Does the CRTC keep generating VBLANKs even when
>>> there are no updates?
>>
>>I don't know. Is this mandatory to have VBLANK events? I mean, the
>>core is using VBLANK events to detect when page flips have been done,
>>that is, when old framebuffers are unused and new ones started to be
>>fetched by the CRTC, but on some HW, VBLANK is not the only way to
>>detect such situations. The question is, are there other situations
>>where VBLANK events are required, or is there an implicit rule stating
>>that a CRTC has to generate VBLANK events at a vrefresh rate even when
>>the display is actually not updated when nothing changes?
>
> I'm not sure how widely relied upon it is, but I'd expect that there's
> a requirement to make sure DRM_IOCTL_WAIT_VBLANK works. I _think_ that
> means the CRTC should generate events at vrefresh rate if there's a
> wait request outstanding that depends on that.

In our case, there's no vrefresh rate, though.  Just completion.

I've been trying to come up with a usecase for vblank events on
writeack, and the best I have is: to enable VNC-style capture of a
complete hardware rendering stack, we could run modesetting against the
writeback connector and do one-shot writebacks when damage comes in.
You would want GL apps to be throttled to the frame capture rate, so X
needs to implement waits for at least a swapinterval of 1 (I don't see
how greater than 1 would make any sense)

However, given that you'd be triggering writebacks on damage, and using
the fence to trigger the next wait for damage and writeback already, I
think you'd use that set of code for Present/DRI2 swapinterval support,
not the current vblank path.

My current inclination would be to throw -EINVAL for userspace vblank
requests on writeback conncetor pipes.


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

Re: [PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-13 Thread Brian Starkey 

Hi Boris,

Sorry lost track of this thread.


On Tue, Jun 06, 2017 at 09:13:00PM +0200, Boris Brezillon wrote:

Hi Brian,

Le Mon, 5 Jun 2017 12:25:50 +0100,
Brian Starkey  a écrit :


Hi Boris,

I can't speak for the HW-specific details, but the writeback part
looks pretty good (and familiar ;-) to me. There certainly seems to be
some scope for code-sharing there, but I think it's fine not to do it
yet.


Agreed.



I've a further query below about the handling of CRTC events.



[...]


>+
>+void vc4_txp_atomic_commit(struct drm_device *dev,
>+  struct drm_atomic_state *old_state)
>+{
>+   struct vc4_dev *vc4 = to_vc4_dev(dev);
>+   struct vc4_txp *txp = vc4->txp;
>+   struct drm_connector_state *conn_state = txp->connector.base.state;
>+   struct drm_display_mode *mode;
>+   struct drm_gem_cma_object *gem;
>+   struct drm_framebuffer *fb;
>+   u32 ctrl = TXP_GO | TXP_EI;
>+
>+   /* Writeback connector is disabled, nothing to do. */
>+   if (!conn_state->crtc)
>+   return;
>+
>+   /* Writeback connector is enabled, but has no FB assigned to it. Fake a
>+* vblank event to complete ->flip_done.
>+*/
>+   if (!conn_state->writeback_job || !conn_state->writeback_job->fb) {
>+   vc4_crtc_eof_event(conn_state->crtc);

I'm not sure about hiding away the one-shot thing like this. If the
CRTC remains "active" from the API point of view, I'd expect it to be
able to keep generating VBLANK events.

I don't know how to do if, but if there were some notion of
"auto-disabling" CRTCs then this quirk would go away, and you'd also
be able to enforce that the CRTC can't be enabled without a writeback
framebuffer.


I don't have a strong opinion on "auto-disabling CRTC" vs "fake VBLANK
events". Note that I'm already faking a VBLANK event when activating
writeback, because there's no such concept at the HVS/TXP level. We
do have EOF (End Of Frame) and writeback done events, but these are
quite independent from the VBLANK events generated by the pixelvalve
block (the block responsible for generating the HSYNC/VSYNC signals).



I'm also not sure how (if?) this works today with a CRTC driving a DSI
command-mode panel. Does the CRTC keep generating VBLANKs even when
there are no updates?


I don't know. Is this mandatory to have VBLANK events? I mean, the
core is using VBLANK events to detect when page flips have been done,
that is, when old framebuffers are unused and new ones started to be
fetched by the CRTC, but on some HW, VBLANK is not the only way to
detect such situations. The question is, are there other situations
where VBLANK events are required, or is there an implicit rule stating
that a CRTC has to generate VBLANK events at a vrefresh rate even when
the display is actually not updated when nothing changes?


I'm not sure how widely relied upon it is, but I'd expect that there's
a requirement to make sure DRM_IOCTL_WAIT_VBLANK works. I _think_ that
means the CRTC should generate events at vrefresh rate if there's a
wait request outstanding that depends on that.

It's not exactly documented though.





>+   return;
>+   }
>+
>+   fb = conn_state->writeback_job->fb;
>+
>+   switch (fb->format->format) {
>+   case DRM_FORMAT_ARGB:
>+   ctrl |= TXP_ALPHA_ENABLE;
>+   case DRM_FORMAT_XRGB:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_ARGB, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+
>+   case DRM_FORMAT_ABGR:
>+   ctrl |= TXP_ALPHA_ENABLE;
>+   case DRM_FORMAT_XBGR:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_ABGR, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+
>+   case DRM_FORMAT_RGBA:
>+   ctrl |= TXP_ALPHA_ENABLE;
>+   case DRM_FORMAT_RGBX:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_RGBA, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+
>+   case DRM_FORMAT_BGRA:
>+   ctrl |= TXP_ALPHA_ENABLE;
>+   case DRM_FORMAT_BGRX:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_BGRA, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+
>+   case DRM_FORMAT_BGR888:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_BGR888, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+
>+   case DRM_FORMAT_RGB888:
>+   ctrl |= VC4_SET_FIELD(TXP_FORMAT_RGB888, TXP_FORMAT) |
>+   VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
>+   break;
>+   default:
>+   WARN_ON(1);
>+   return;
>+   }
>+
>+   if (!(ctrl & TXP_ALPHA_ENABLE))
>+   ctrl |= TXP_ALPHA_INVERT;
>+
>+   mode = _state->crtc->state->adjusted_mode;
>+   gem = drm_fb_cma_get_gem_obj(fb, 0);
>+   TXP_WRITE(TXP_DST_PTR, gem->paddr + fb->offsets[0]);
>+   TXP_WRITE(TXP_DST_PITCH, fb->pitches[0]);
>+   TXP_WRITE(TXP_DIM,
>+

Re: [PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-06 Thread Boris Brezillon 
Hi Brian,

Le Mon, 5 Jun 2017 12:25:50 +0100,
Brian Starkey  a écrit :

> Hi Boris,
> 
> I can't speak for the HW-specific details, but the writeback part
> looks pretty good (and familiar ;-) to me. There certainly seems to be
> some scope for code-sharing there, but I think it's fine not to do it
> yet.

Agreed.

> 
> I've a further query below about the handling of CRTC events.
> 

[...]

> >+
> >+void vc4_txp_atomic_commit(struct drm_device *dev,
> >+   struct drm_atomic_state *old_state)
> >+{
> >+struct vc4_dev *vc4 = to_vc4_dev(dev);
> >+struct vc4_txp *txp = vc4->txp;
> >+struct drm_connector_state *conn_state = txp->connector.base.state;
> >+struct drm_display_mode *mode;
> >+struct drm_gem_cma_object *gem;
> >+struct drm_framebuffer *fb;
> >+u32 ctrl = TXP_GO | TXP_EI;
> >+
> >+/* Writeback connector is disabled, nothing to do. */
> >+if (!conn_state->crtc)
> >+return;
> >+
> >+/* Writeback connector is enabled, but has no FB assigned to it. Fake a
> >+ * vblank event to complete ->flip_done.
> >+ */
> >+if (!conn_state->writeback_job || !conn_state->writeback_job->fb) {
> >+vc4_crtc_eof_event(conn_state->crtc);  
> 
> I'm not sure about hiding away the one-shot thing like this. If the
> CRTC remains "active" from the API point of view, I'd expect it to be
> able to keep generating VBLANK events.
> 
> I don't know how to do if, but if there were some notion of
> "auto-disabling" CRTCs then this quirk would go away, and you'd also
> be able to enforce that the CRTC can't be enabled without a writeback
> framebuffer.

I don't have a strong opinion on "auto-disabling CRTC" vs "fake VBLANK
events". Note that I'm already faking a VBLANK event when activating
writeback, because there's no such concept at the HVS/TXP level. We
do have EOF (End Of Frame) and writeback done events, but these are
quite independent from the VBLANK events generated by the pixelvalve
block (the block responsible for generating the HSYNC/VSYNC signals).

> 
> I'm also not sure how (if?) this works today with a CRTC driving a DSI
> command-mode panel. Does the CRTC keep generating VBLANKs even when
> there are no updates?

I don't know. Is this mandatory to have VBLANK events? I mean, the
core is using VBLANK events to detect when page flips have been done,
that is, when old framebuffers are unused and new ones started to be
fetched by the CRTC, but on some HW, VBLANK is not the only way to
detect such situations. The question is, are there other situations
where VBLANK events are required, or is there an implicit rule stating
that a CRTC has to generate VBLANK events at a vrefresh rate even when
the display is actually not updated when nothing changes?

> 
> >+return;
> >+}
> >+
> >+fb = conn_state->writeback_job->fb;
> >+
> >+switch (fb->format->format) {
> >+case DRM_FORMAT_ARGB:
> >+ctrl |= TXP_ALPHA_ENABLE;
> >+case DRM_FORMAT_XRGB:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_ARGB, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+
> >+case DRM_FORMAT_ABGR:
> >+ctrl |= TXP_ALPHA_ENABLE;
> >+case DRM_FORMAT_XBGR:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_ABGR, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+
> >+case DRM_FORMAT_RGBA:
> >+ctrl |= TXP_ALPHA_ENABLE;
> >+case DRM_FORMAT_RGBX:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_RGBA, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+
> >+case DRM_FORMAT_BGRA:
> >+ctrl |= TXP_ALPHA_ENABLE;
> >+case DRM_FORMAT_BGRX:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_BGRA, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+
> >+case DRM_FORMAT_BGR888:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_BGR888, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+
> >+case DRM_FORMAT_RGB888:
> >+ctrl |= VC4_SET_FIELD(TXP_FORMAT_RGB888, TXP_FORMAT) |
> >+VC4_SET_FIELD(0xf, TXP_BYTE_ENABLE);
> >+break;
> >+default:
> >+WARN_ON(1);
> >+return;
> >+}
> >+
> >+if (!(ctrl & TXP_ALPHA_ENABLE))
> >+ctrl |= TXP_ALPHA_INVERT;
> >+
> >+mode = _state->crtc->state->adjusted_mode;
> >+gem = drm_fb_cma_get_gem_obj(fb, 0);
> >+TXP_WRITE(TXP_DST_PTR, gem->paddr + fb->offsets[0]);
> >+TXP_WRITE(TXP_DST_PITCH, fb->pitches[0]);
> >+TXP_WRITE(TXP_DIM,
> >+  VC4_SET_FIELD(mode->hdisplay, TXP_WIDTH) |
> >+  VC4_SET_FIELD(mode->vdisplay, TXP_HEIGHT));
> >+
> >+TXP_WRITE(TXP_DST_CTRL, ctrl);
> >+
> >+

Re: [PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-05 Thread Brian Starkey 

Hi Boris,

I can't speak for the HW-specific details, but the writeback part
looks pretty good (and familiar ;-) to me. There certainly seems to be
some scope for code-sharing there, but I think it's fine not to do it
yet.

I've a further query below about the handling of CRTC events.

On Fri, Jun 02, 2017 at 10:32:10AM +0200, Boris Brezillon wrote:

The TXP block is providing writeback support. Add a driver to expose this
feature.

Supporting this engine requires reworking the CRTC because it's not using
the usual 'HVS -> PV -> VC4 Encoder' scheme. Instead, the TXP block is
directly connected to HVS FIFO2, and the PV is completely bypassed.

In order to make things work, we have to detect when the TXP is enabled,
avoid enabling the PV when this is the case, and generate fake VBLANK
events when the TXP is done writing the frame back to memory.

Signed-off-by: Boris Brezillon 
---


[snip]


diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
new file mode 100644
index ..1886e031bef3
--- /dev/null
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -0,0 +1,509 @@
+/*
+ * Copyright © 2017 Broadcom
+ *
+ * Author: Boris Brezillon 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "vc4_drv.h"
+#include "vc4_regs.h"
+
+/* Base address of the output.  Raster formats must be 4-byte aligned,
+ * T and LT must be 16-byte aligned or maybe utile-aligned (docs are
+ * inconsistent, but probably utile).
+ */
+#define TXP_DST_PTR0x00
+
+/* Pitch in bytes for raster images, 16-byte aligned.  For tiled, it's
+ * the width in tiles.
+ */
+#define TXP_DST_PITCH  0x04
+/* For T-tiled imgaes, DST_PITCH should be the number of tiles wide,
+ * shifted up.
+ */
+# define TXP_T_TILE_WIDTH_SHIFT7
+/* For LT-tiled images, DST_PITCH should be the number of utiles wide,
+ * shifted up.
+ */
+# define TXP_LT_TILE_WIDTH_SHIFT   4
+
+/* Pre-rotation width/height of the image.  Must match HVS config.
+ *
+ * If TFORMAT and 32-bit, limit is 1920 for 32-bit and 3840 to 16-bit
+ * and width/height must be tile or utile-aligned as appropriate.  If
+ * transposing (rotating), width is limited to 1920.
+ *
+ * Height is limited to various numbers between 4088 and 4095.  I'd
+ * just use 4088 to be safe.
+ */
+#define TXP_DIM0x08
+# define TXP_HEIGHT_SHIFT  16
+# define TXP_HEIGHT_MASK   GENMASK(31, 16)
+# define TXP_WIDTH_SHIFT   0
+# define TXP_WIDTH_MASKGENMASK(15, 0)
+
+#define TXP_DST_CTRL   0x0c
+/* These bits are set to 0x54 */
+#define TXP_PILOT_SHIFT24
+#define TXP_PILOT_MASK GENMASK(31, 24)
+/* Bits 22-23 are set to 0x01 */
+#define TXP_VERSION_SHIFT  22
+#define TXP_VERSION_MASK   GENMASK(23, 22)
+
+/* Powers down the internal memory. */
+# define TXP_POWERDOWN BIT(21)
+
+/* Enables storing the alpha component in /, instead of
+ * filling with ~ALPHA_INVERT.
+ */
+# define TXP_ALPHA_ENABLE  BIT(20)
+
+/* 4 bits, each enables stores for a channel in each set of 4 bytes.
+ * Set to 0xf for normal operation.
+ */
+# define TXP_BYTE_ENABLE_SHIFT 16
+# define TXP_BYTE_ENABLE_MASK  GENMASK(19, 16)
+
+/* Debug: Generate VSTART again at EOF. */
+# define TXP_VSTART_AT_EOF BIT(15)
+
+/* Debug: Terminate the current frame immediately.  Stops AXI
+ * writes.
+ */
+# define TXP_ABORT BIT(14)
+
+# define TXP_DITHERBIT(13)
+
+/* Inverts alpha if TXP_ALPHA_ENABLE, chooses fill value for
+ * !TXP_ALPHA_ENABLE.
+ */
+# define TXP_ALPHA_INVERT

[PATCH 5/7] drm/vc4: Add support for the TXP (transposer) block

2017-06-02 Thread Boris Brezillon 
The TXP block is providing writeback support. Add a driver to expose this
feature.

Supporting this engine requires reworking the CRTC because it's not using
the usual 'HVS -> PV -> VC4 Encoder' scheme. Instead, the TXP block is
directly connected to HVS FIFO2, and the PV is completely bypassed.

In order to make things work, we have to detect when the TXP is enabled,
avoid enabling the PV when this is the case, and generate fake VBLANK
events when the TXP is done writing the frame back to memory.

Signed-off-by: Boris Brezillon 
---
 drivers/gpu/drm/vc4/Makefile  |   1 +
 drivers/gpu/drm/vc4/vc4_crtc.c| 126 --
 drivers/gpu/drm/vc4/vc4_debugfs.c |   1 +
 drivers/gpu/drm/vc4/vc4_drv.c |   1 +
 drivers/gpu/drm/vc4/vc4_drv.h |  10 +
 drivers/gpu/drm/vc4/vc4_kms.c |   7 +
 drivers/gpu/drm/vc4/vc4_regs.h|   1 +
 drivers/gpu/drm/vc4/vc4_txp.c | 509 ++
 8 files changed, 634 insertions(+), 22 deletions(-)
 create mode 100644 drivers/gpu/drm/vc4/vc4_txp.c

diff --git a/drivers/gpu/drm/vc4/Makefile b/drivers/gpu/drm/vc4/Makefile
index 61f45d122bd0..660b3363400f 100644
--- a/drivers/gpu/drm/vc4/Makefile
+++ b/drivers/gpu/drm/vc4/Makefile
@@ -18,6 +18,7 @@ vc4-y := \
vc4_plane.o \
vc4_render_cl.o \
vc4_trace_points.o \
+   vc4_txp.o \
vc4_v3d.o \
vc4_validate.o \
vc4_validate_shaders.o
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index df1d81533076..564fd795803d 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -66,6 +66,7 @@ struct vc4_crtc_state {
struct drm_crtc_state base;
/* Dlist area for this CRTC configuration. */
struct drm_mm_node mm;
+   bool feed_txp;
 };
 
 static inline struct vc4_crtc *
@@ -369,10 +370,8 @@ static struct drm_encoder *vc4_get_crtc_encoder(struct 
drm_crtc *crtc)
return NULL;
 }
 
-static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
+static void vc4_crtc_config_pv(struct drm_crtc *crtc)
 {
-   struct drm_device *dev = crtc->dev;
-   struct vc4_dev *vc4 = to_vc4_dev(dev);
struct drm_encoder *encoder = vc4_get_crtc_encoder(crtc);
struct vc4_encoder *vc4_encoder = to_vc4_encoder(encoder);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
@@ -383,12 +382,6 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
bool is_dsi = (vc4_encoder->type == VC4_ENCODER_TYPE_DSI0 ||
   vc4_encoder->type == VC4_ENCODER_TYPE_DSI1);
u32 format = is_dsi ? PV_CONTROL_FORMAT_DSIV_24 : PV_CONTROL_FORMAT_24;
-   bool debug_dump_regs = false;
-
-   if (debug_dump_regs) {
-   DRM_INFO("CRTC %d regs before:\n", drm_crtc_index(crtc));
-   vc4_crtc_dump_regs(vc4_crtc);
-   }
 
/* Reset the PV fifo. */
CRTC_WRITE(PV_CONTROL, 0);
@@ -464,6 +457,51 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 PV_CONTROL_CLK_SELECT) |
   PV_CONTROL_FIFO_CLR |
   PV_CONTROL_EN);
+}
+
+static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc->dev;
+   struct vc4_dev *vc4 = to_vc4_dev(dev);
+   struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
+   struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
+   struct drm_display_mode *mode = >state->adjusted_mode;
+   bool interlace = mode->flags & DRM_MODE_FLAG_INTERLACE;
+   bool debug_dump_regs = false;
+
+   if (debug_dump_regs) {
+   DRM_INFO("CRTC %d regs before:\n", drm_crtc_index(crtc));
+   vc4_crtc_dump_regs(vc4_crtc);
+   }
+
+   if (vc4_crtc->channel == 2) {
+   u32 dispctrl, olddispctrl;
+
+   olddispctrl = HVS_READ(SCALER_DISPCTRL);
+   dispctrl = olddispctrl & ~SCALER_DISPCTRL_DSP3_MUX_MASK;
+
+   /* SCALER_DISPCTRL_DSP3 = X, where X < 2 means 'connect DSP3 to
+* FIFO X'.
+* SCALER_DISPCTRL_DSP3 = 3 means 'disable DSP 3'.
+*
+* DSP3 is connected to FIFO2 unless the transposer is
+* enabled. In this case, FIFO 2 is directly accessed by the
+* TXP IP, and we need to disable the FIFO2 -> pixelvalve1
+* route.
+*/
+   if (vc4_state->feed_txp)
+   dispctrl |= VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX);
+   else
+   dispctrl |= VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX);
+
+   /* Reconfigure the DSP3 mux if required. */
+   if (olddispctrl != dispctrl)
+   HVS_WRITE(SCALER_DISPCTRL, dispctrl);
+   }
+
+   /* Only configure the pixelvalve if we're not feeding the TXP block. */
+   if (!vc4_state->feed_txp)
+