[PATCH v2 7/8] drm/vc4: Add support for the transposer block
Eric Anholt
eric at anholt.net
Fri Jun 29 20:35:04 UTC 2018
Boris Brezillon <boris.brezillon at bootlin.com> writes:
> From: Boris Brezillon <boris.brezillon at free-electrons.com>
>
> The transposer block is providing support for mem-to-mem composition,
> which is exposed as a drm_writeback connector in DRM.
>
> Add a driver to support this feature.
>
> Signed-off-by: Boris Brezillon <boris.brezillon at free-electrons.com>
> +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 = &crtc->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;
> + u32 dsp3_mux;
> +
> + /*
> + * 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 prevent disable the
s/prevent //
> + * FIFO2 -> pixelvalve1 route.
> + */
> + if (vc4_state->feed_txp)
> + dsp3_mux = VC4_SET_FIELD(3, SCALER_DISPCTRL_DSP3_MUX);
> + else
> + dsp3_mux = VC4_SET_FIELD(2, SCALER_DISPCTRL_DSP3_MUX);
> +
> + /* Reconfigure the DSP3 mux if required. */
> + dispctrl = HVS_READ(SCALER_DISPCTRL);
> + if ((dispctrl & SCALER_DISPCTRL_DSP3_MUX_MASK) != dsp3_mux) {
> + dispctrl &= ~SCALER_DISPCTRL_DSP3_MUX_MASK;
> + HVS_WRITE(SCALER_DISPCTRL, dispctrl | dsp3_mux);
> + }
This is fine, but you could also skip the matching mux check here -- the
read is the expensive part.
> + }
> +
> + if (!vc4_state->feed_txp)
> + vc4_crtc_config_pv(crtc);
>
> HVS_WRITE(SCALER_DISPBKGNDX(vc4_crtc->channel),
> SCALER_DISPBKGND_AUTOHS |
> @@ -499,6 +539,13 @@ static void vc4_crtc_atomic_disable(struct drm_crtc *crtc,
> }
> }
>
> +void vc4_crtc_txp_armed(struct drm_crtc_state *state)
> +{
> + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state);
> +
> + vc4_state->txp_armed = true;
> +}
> +
> static void vc4_crtc_update_dlist(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -514,8 +561,11 @@ static void vc4_crtc_update_dlist(struct drm_crtc *crtc)
> WARN_ON(drm_crtc_vblank_get(crtc) != 0);
>
> spin_lock_irqsave(&dev->event_lock, flags);
> - vc4_crtc->event = crtc->state->event;
> - crtc->state->event = NULL;
> +
> + if (!vc4_state->feed_txp || vc4_state->txp_armed) {
> + vc4_crtc->event = crtc->state->event;
> + crtc->state->event = NULL;
> + }
>
> HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel),
> vc4_state->mm.start);
> @@ -533,8 +583,8 @@ static void vc4_crtc_atomic_enable(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 drm_crtc_state *state = crtc->state;
> - struct drm_display_mode *mode = &state->adjusted_mode;
> + struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc->state);
> + struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>
> require_hvs_enabled(dev);
>
> @@ -546,15 +596,21 @@ static void vc4_crtc_atomic_enable(struct drm_crtc *crtc,
>
> /* Turn on the scaler, which will wait for vstart to start
> * compositing.
> + * When feeding the transposer, we should operate in oneshot
> + * mode.
> */
> HVS_WRITE(SCALER_DISPCTRLX(vc4_crtc->channel),
> VC4_SET_FIELD(mode->hdisplay, SCALER_DISPCTRLX_WIDTH) |
> VC4_SET_FIELD(mode->vdisplay, SCALER_DISPCTRLX_HEIGHT) |
> - SCALER_DISPCTRLX_ENABLE);
> + SCALER_DISPCTRLX_ENABLE |
> + (vc4_state->feed_txp ? SCALER_DISPCTRLX_ONESHOT : 0));
>
> - /* Turn on the pixel valve, which will emit the vstart signal. */
> - CRTC_WRITE(PV_V_CONTROL,
> - CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
> + /* When feeding the composer block the pixelvalve is unneeded and
"transposer block"? composer block made me think HVS.
> + * should not be enabled.
> + */
> + if (!vc4_state->feed_txp)
> + CRTC_WRITE(PV_V_CONTROL,
> + CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
> }
>
> static enum drm_mode_status vc4_crtc_mode_valid(struct drm_crtc *crtc,
> @@ -579,8 +635,10 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
> struct drm_plane *plane;
> unsigned long flags;
> const struct drm_plane_state *plane_state;
> + struct drm_connector *conn;
> + struct drm_connector_state *conn_state;
> u32 dlist_count = 0;
> - int ret;
> + int ret, i;
>
> /* The pixelvalve can only feed one encoder (and encoders are
> * 1:1 with connectors.)
> @@ -600,6 +658,22 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
> if (ret)
> return ret;
>
> + state->no_vblank = false;
> + for_each_new_connector_in_state(state->state, conn, conn_state, i) {
> + if (conn_state->crtc != crtc)
> + continue;
> +
> + /* The writeback connector is implemented using the transposer
> + * block which is directly taking its data from the HVS FIFO.
> + */
> + if (conn->connector_type == DRM_MODE_CONNECTOR_WRITEBACK) {
> + state->no_vblank = true;
> + vc4_state->feed_txp = true;
> + }
> +
> + break;
> + }
> +
> return 0;
> }
>
> @@ -713,7 +787,8 @@ static void vc4_crtc_handle_page_flip(struct vc4_crtc *vc4_crtc)
>
> spin_lock_irqsave(&dev->event_lock, flags);
> if (vc4_crtc->event &&
> - (vc4_state->mm.start == HVS_READ(SCALER_DISPLACTX(chan)))) {
> + (vc4_state->mm.start == HVS_READ(SCALER_DISPLACTX(chan)) ||
> + vc4_state->feed_txp)) {
Can vc4_crtc->event even be set if vc4_state->feed_txp?
> diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
> index d6864fa4bd14..744a689751f0 100644
> --- a/drivers/gpu/drm/vc4/vc4_regs.h
> +++ b/drivers/gpu/drm/vc4/vc4_regs.h
> @@ -330,6 +330,7 @@
> #define SCALER_DISPCTRL0 0x00000040
> # define SCALER_DISPCTRLX_ENABLE BIT(31)
> # define SCALER_DISPCTRLX_RESET BIT(30)
> +
> /* Generates a single frame when VSTART is seen and stops at the last
> * pixel read from the FIFO.
> */
stray hunk?
> +static void vc4_txp_connector_atomic_commit(struct drm_connector *conn,
> + struct drm_connector_state *conn_state)
> +{
> + struct vc4_txp *txp = connector_to_vc4_txp(conn);
> + struct drm_gem_cma_object *gem;
> + struct drm_display_mode *mode;
> + struct drm_framebuffer *fb;
> + u32 ctrl = TXP_GO | TXP_VSTART_AT_EOF | TXP_EI;
> +
> + if (WARN_ON(!conn_state->writeback_job ||
> + !conn_state->writeback_job->fb))
> + return;
> +
> + mode = &conn_state->crtc->state->adjusted_mode;
> + fb = conn_state->writeback_job->fb;
> +
> + switch (fb->format->format) {
> + case DRM_FORMAT_ARGB8888:
> + ctrl |= TXP_ALPHA_ENABLE;
Optional suggestion: Have the txp_formats[] table be a struct with these
register values in it.
All my feedback seems really minor, so with whatever components you like
integrated, feel free to add:
Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180629/7381a74a/attachment.sig>
More information about the dri-devel
mailing list