| Message ID | 20220209095350.2104049-1-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/rockchip: RK356x VOP2 support | expand |
Hi Sascha
tested full v5 Series + 3 clk-Patches from v4 on 5.17-rc2 on my rk3568 based Bananapi R2 Pro
1280x720-32@60Hz
1920x1080-32@60Hz
3840x2160-32@60Hz
with fb console
Tested-by: Frank Wunderlich <frank-w@public-files.de>
regards Frank
Hi Sascha
tested full v5 Series + 3 clk-Patches from v4 on 5.17-rc2 on my rk3568 based Bananapi R2 Pro
1280x720-32@60Hz
1920x1080-32@60Hz
3840x2160-32@60Hz
with fb console
Tested-by: Frank Wunderlich <frank-w@public-files.de>
regards Frank
09.02.2022 12:53, Sascha Hauer пишет: > + np = of_graph_get_remote_node(dev->of_node, i, -1); > + if (!np) { > + printk("%s: No remote for vp%d\n", __func__, i); drm_dbg
09.02.2022 12:53, Sascha Hauer пишет: > +static irqreturn_t vop2_isr(int irq, void *data) > +{ > + struct vop2 *vop2 = data; > + const struct vop2_data *vop2_data = vop2->data; > + uint32_t axi_irqs[VOP2_SYS_AXI_BUS_NUM]; > + int ret = IRQ_NONE; > + int i; > + > + /* > + * The irq is shared with the iommu. If the runtime-pm state of the > + * vop2-device is disabled the irq has to be targeted at the iommu. > + */ > + if (!pm_runtime_get_if_in_use(vop2->dev)) > + return IRQ_NONE; > + > + for (i = 0; i < vop2_data->nr_vps; i++) { > + struct vop2_video_port *vp = &vop2->vps[i]; > + struct drm_crtc *crtc = &vp->crtc; > + uint32_t irqs; > + > + irqs = vop2_readl(vop2, RK3568_VP_INT_STATUS(vp->id)); > + vop2_writel(vop2, RK3568_VP_INT_CLR(vp->id), irqs << 16 | irqs); > + > + if (irqs & VP_INT_DSP_HOLD_VALID) { > + complete(&vp->dsp_hold_completion); > + ret = IRQ_HANDLED; > + } > + > + if (irqs & VP_INT_FS_FIELD) { > + unsigned long flags; > + > + drm_crtc_handle_vblank(crtc); > + spin_lock_irqsave(&crtc->dev->event_lock, flags); IRQ is disabled inside of ISR(), no need to save/restore the flags.
09.02.2022 12:53, Sascha Hauer пишет: > @@ -31,6 +31,9 @@ struct rockchip_crtc_state { > int output_bpc; > int output_flags; > bool enable_afbc; > + uint32_t bus_format; > + u32 bus_flags; > + int color_space; The variable types could be cleaned up like s/uint32_t/u32/ all over this patch. I'd also remove signes from all variables that should be unsigned.
09.02.2022 12:53, Sascha Hauer пишет: > +static void vop2_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) > +{ > + struct drm_plane_state *pstate = plane->state; > + struct drm_crtc *crtc = pstate->crtc; > + struct vop2_win *win = to_vop2_win(plane); > + struct vop2_video_port *vp = to_vop2_video_port(crtc); > + struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode; > + struct vop2 *vop2 = win->vop2; > + struct drm_framebuffer *fb = pstate->fb; > + uint32_t bpp = fb->format->cpp[0] * 8; > + uint32_t actual_w, actual_h, dsp_w, dsp_h; > + uint32_t act_info, dsp_info; > + uint32_t format; > + uint32_t afbc_format; > + uint32_t rb_swap; > + uint32_t uv_swap; > + struct drm_rect *src = &pstate->src; > + struct drm_rect *dest = &pstate->dst; > + uint32_t afbc_tile_num; > + uint32_t transform_offset; > + bool dither_up; > + bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; > + bool ymirror = pstate->rotation & DRM_MODE_REFLECT_Y; > + bool rotate_270 = pstate->rotation & DRM_MODE_ROTATE_270; > + bool rotate_90 = pstate->rotation & DRM_MODE_ROTATE_90; > + struct rockchip_gem_object *rk_obj; > + unsigned long offset; > + bool afbc_en; > + dma_addr_t yrgb_mst; > + dma_addr_t uv_mst; > + > + /* > + * can't update plane when vop2 is disabled. > + */ > + if (WARN_ON(!crtc)) > + return; > + > + if (!pstate->visible) { > + vop2_plane_atomic_disable(plane, state); > + return; > + } > + > + afbc_en = rockchip_afbc(plane, fb->modifier); > + > + offset = (src->x1 >> 16) * fb->format->cpp[0]; > + > + /* > + * AFBC HDR_PTR must set to the zero offset of the framebuffer. > + */ > + if (afbc_en) > + offset = 0; > + else if (pstate->rotation & DRM_MODE_REFLECT_Y) > + offset += ((src->y2 >> 16) - 1) * fb->pitches[0]; > + else > + offset += (src->y1 >> 16) * fb->pitches[0]; > + > + rk_obj = to_rockchip_obj(fb->obj[0]); > + > + yrgb_mst = rk_obj->dma_addr + offset + fb->offsets[0]; > + if (fb->format->is_yuv) { > + int hsub = fb->format->hsub; > + int vsub = fb->format->vsub; > + > + offset = (src->x1 >> 16) * fb->format->cpp[1] / hsub; > + offset += (src->y1 >> 16) * fb->pitches[1] / vsub; > + > + if ((pstate->rotation & DRM_MODE_REFLECT_Y) && !afbc_en) > + offset += fb->pitches[1] * ((pstate->src_h >> 16) - 2) / vsub; > + > + rk_obj = to_rockchip_obj(fb->obj[0]); > + uv_mst = rk_obj->dma_addr + offset + fb->offsets[1]; > + } > + > + actual_w = drm_rect_width(src) >> 16; > + actual_h = drm_rect_height(src) >> 16; > + dsp_w = drm_rect_width(dest); > + > + if (dest->x1 + dsp_w > adjusted_mode->hdisplay) { > + drm_err(vop2->drm, "vp%d %s dest->x1[%d] + dsp_w[%d] exceed mode hdisplay[%d]\n", > + vp->id, win->data->name, dest->x1, dsp_w, adjusted_mode->hdisplay); > + dsp_w = adjusted_mode->hdisplay - dest->x1; > + if (dsp_w < 4) > + dsp_w = 4; > + actual_w = dsp_w * actual_w / drm_rect_width(dest); > + } > + > + dsp_h = drm_rect_height(dest); > + > + if (dest->y1 + dsp_h > adjusted_mode->vdisplay) { > + drm_err(vop2->drm, "vp%d %s dest->y1[%d] + dsp_h[%d] exceed mode vdisplay[%d]\n", > + vp->id, win->data->name, dest->y1, dsp_h, adjusted_mode->vdisplay); > + dsp_h = adjusted_mode->vdisplay - dest->y1; > + if (dsp_h < 4) > + dsp_h = 4; > + actual_h = dsp_h * actual_h / drm_rect_height(dest); > + } > + > + /* > + * This is workaround solution for IC design: > + * esmart can't support scale down when actual_w % 16 == 1. > + */ > + if (!(win->data->feature & WIN_FEATURE_AFBDC)) { > + if (actual_w > dsp_w && (actual_w & 0xf) == 1) { > + drm_err(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n", vp->id, win->data->name, actual_w); > + actual_w -= 1; > + } > + } > + > + if (afbc_en && actual_w % 4) { > + drm_err(vop2->drm, "vp%d %s actual_w[%d] should align as 4 pixel when enable afbc\n", > + vp->id, win->data->name, actual_w); > + actual_w = ALIGN_DOWN(actual_w, 4); > + } > + > + act_info = (actual_h - 1) << 16 | ((actual_w - 1) & 0xffff); > + dsp_info = (dsp_h - 1) << 16 | ((dsp_w - 1) & 0xffff); > + > + format = vop2_convert_format(fb->format->format); > + > + drm_dbg(vop2->drm, "vp%d update %s[%dx%d->%dx%d@%dx%d] fmt[%p4cc_%s] addr[%pad]\n", > + vp->id, win->data->name, actual_w, actual_h, dsp_w, dsp_h, > + dest->x1, dest->y1, > + &fb->format->format, > + afbc_en ? "AFBC" : "", &yrgb_mst); > + > + if (afbc_en) { > + uint32_t stride; > + > + /* the afbc superblock is 16 x 16 */ > + afbc_format = vop2_convert_afbc_format(fb->format->format); > + > + /* Enable color transform for YTR */ > + if (fb->modifier & AFBC_FORMAT_MOD_YTR) > + afbc_format |= (1 << 4); > + > + afbc_tile_num = ALIGN(actual_w, 16) >> 4; > + > + /* > + * AFBC pic_vir_width is count by pixel, this is different > + * with WIN_VIR_STRIDE. > + */ > + stride = (fb->pitches[0] << 3) / bpp; > + if ((stride & 0x3f) && (xmirror || rotate_90 || rotate_270)) > + drm_err(vop2->drm, "vp%d %s stride[%d] must align as 64 pixel when enable xmirror/rotate_90/rotate_270[0x%x]\n", > + vp->id, win->data->name, stride, pstate->rotation); > + > + rb_swap = vop2_afbc_rb_swap(fb->format->format); > + uv_swap = vop2_afbc_uv_swap(fb->format->format); > + /* > + * This is a workaround for crazy IC design, Cluster > + * and Esmart/Smart use different format configuration map: > + * YUV420_10BIT: 0x10 for Cluster, 0x14 for Esmart/Smart. > + * > + * This is one thing we can make the convert simple: > + * AFBCD decode all the YUV data to YUV444. So we just > + * set all the yuv 10 bit to YUV444_10. > + */ > + if (fb->format->is_yuv && (bpp == 10)) > + format = VOP2_CLUSTER_YUV444_10; > + > + if (vop2_cluster_window(win)) > + vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1); > + vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format); > + vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap); > + vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap); > + vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0); > + vop2_win_write(win, VOP2_WIN_AFBC_BLOCK_SPLIT_EN, 0); > + if (pstate->rotation & (DRM_MODE_ROTATE_270 | DRM_MODE_ROTATE_90)) { > + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 0); > + transform_offset = vop2_afbc_transform_offset(pstate, false); > + } else { > + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 1); > + transform_offset = vop2_afbc_transform_offset(pstate, true); > + } > + vop2_win_write(win, VOP2_WIN_AFBC_HDR_PTR, yrgb_mst); > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_SIZE, act_info); > + vop2_win_write(win, VOP2_WIN_AFBC_TRANSFORM_OFFSET, transform_offset); > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_OFFSET, ((src->x1 >> 16) | src->y1)); > + vop2_win_write(win, VOP2_WIN_AFBC_DSP_OFFSET, (dest->x1 | (dest->y1 << 16))); > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_VIR_WIDTH, stride); > + vop2_win_write(win, VOP2_WIN_AFBC_TILE_NUM, afbc_tile_num); > + vop2_win_write(win, VOP2_WIN_XMIRROR, xmirror); The xmirror's variable type is specified as bool, but it's not true/false because in the above code: bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; I don't see how vop2_win_write() could work properly. Or am I missing something?
On Wed, Feb 16, 2022 at 01:39:33PM +0300, Dmitry Osipenko wrote: > 09.02.2022 12:53, Sascha Hauer пишет: > > +static void vop2_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) > > +{ > > + struct drm_plane_state *pstate = plane->state; > > + struct drm_crtc *crtc = pstate->crtc; > > + struct vop2_win *win = to_vop2_win(plane); > > + struct vop2_video_port *vp = to_vop2_video_port(crtc); > > + struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode; > > + struct vop2 *vop2 = win->vop2; > > + struct drm_framebuffer *fb = pstate->fb; > > + uint32_t bpp = fb->format->cpp[0] * 8; > > + uint32_t actual_w, actual_h, dsp_w, dsp_h; > > + uint32_t act_info, dsp_info; > > + uint32_t format; > > + uint32_t afbc_format; > > + uint32_t rb_swap; > > + uint32_t uv_swap; > > + struct drm_rect *src = &pstate->src; > > + struct drm_rect *dest = &pstate->dst; > > + uint32_t afbc_tile_num; > > + uint32_t transform_offset; > > + bool dither_up; > > + bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; > > + bool ymirror = pstate->rotation & DRM_MODE_REFLECT_Y; > > + bool rotate_270 = pstate->rotation & DRM_MODE_ROTATE_270; > > + bool rotate_90 = pstate->rotation & DRM_MODE_ROTATE_90; > > + struct rockchip_gem_object *rk_obj; > > + unsigned long offset; > > + bool afbc_en; > > + dma_addr_t yrgb_mst; > > + dma_addr_t uv_mst; > > + > > + /* > > + * can't update plane when vop2 is disabled. > > + */ > > + if (WARN_ON(!crtc)) > > + return; > > + > > + if (!pstate->visible) { > > + vop2_plane_atomic_disable(plane, state); > > + return; > > + } > > + > > + afbc_en = rockchip_afbc(plane, fb->modifier); > > + > > + offset = (src->x1 >> 16) * fb->format->cpp[0]; > > + > > + /* > > + * AFBC HDR_PTR must set to the zero offset of the framebuffer. > > + */ > > + if (afbc_en) > > + offset = 0; > > + else if (pstate->rotation & DRM_MODE_REFLECT_Y) > > + offset += ((src->y2 >> 16) - 1) * fb->pitches[0]; > > + else > > + offset += (src->y1 >> 16) * fb->pitches[0]; > > + > > + rk_obj = to_rockchip_obj(fb->obj[0]); > > + > > + yrgb_mst = rk_obj->dma_addr + offset + fb->offsets[0]; > > + if (fb->format->is_yuv) { > > + int hsub = fb->format->hsub; > > + int vsub = fb->format->vsub; > > + > > + offset = (src->x1 >> 16) * fb->format->cpp[1] / hsub; > > + offset += (src->y1 >> 16) * fb->pitches[1] / vsub; > > + > > + if ((pstate->rotation & DRM_MODE_REFLECT_Y) && !afbc_en) > > + offset += fb->pitches[1] * ((pstate->src_h >> 16) - 2) / vsub; > > + > > + rk_obj = to_rockchip_obj(fb->obj[0]); > > + uv_mst = rk_obj->dma_addr + offset + fb->offsets[1]; > > + } > > + > > + actual_w = drm_rect_width(src) >> 16; > > + actual_h = drm_rect_height(src) >> 16; > > + dsp_w = drm_rect_width(dest); > > + > > + if (dest->x1 + dsp_w > adjusted_mode->hdisplay) { > > + drm_err(vop2->drm, "vp%d %s dest->x1[%d] + dsp_w[%d] exceed mode hdisplay[%d]\n", > > + vp->id, win->data->name, dest->x1, dsp_w, adjusted_mode->hdisplay); > > + dsp_w = adjusted_mode->hdisplay - dest->x1; > > + if (dsp_w < 4) > > + dsp_w = 4; > > + actual_w = dsp_w * actual_w / drm_rect_width(dest); > > + } > > + > > + dsp_h = drm_rect_height(dest); > > + > > + if (dest->y1 + dsp_h > adjusted_mode->vdisplay) { > > + drm_err(vop2->drm, "vp%d %s dest->y1[%d] + dsp_h[%d] exceed mode vdisplay[%d]\n", > > + vp->id, win->data->name, dest->y1, dsp_h, adjusted_mode->vdisplay); > > + dsp_h = adjusted_mode->vdisplay - dest->y1; > > + if (dsp_h < 4) > > + dsp_h = 4; > > + actual_h = dsp_h * actual_h / drm_rect_height(dest); > > + } > > + > > + /* > > + * This is workaround solution for IC design: > > + * esmart can't support scale down when actual_w % 16 == 1. > > + */ > > + if (!(win->data->feature & WIN_FEATURE_AFBDC)) { > > + if (actual_w > dsp_w && (actual_w & 0xf) == 1) { > > + drm_err(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n", vp->id, win->data->name, actual_w); > > + actual_w -= 1; > > + } > > + } > > + > > + if (afbc_en && actual_w % 4) { > > + drm_err(vop2->drm, "vp%d %s actual_w[%d] should align as 4 pixel when enable afbc\n", > > + vp->id, win->data->name, actual_w); > > + actual_w = ALIGN_DOWN(actual_w, 4); > > + } > > + > > + act_info = (actual_h - 1) << 16 | ((actual_w - 1) & 0xffff); > > + dsp_info = (dsp_h - 1) << 16 | ((dsp_w - 1) & 0xffff); > > + > > + format = vop2_convert_format(fb->format->format); > > + > > + drm_dbg(vop2->drm, "vp%d update %s[%dx%d->%dx%d@%dx%d] fmt[%p4cc_%s] addr[%pad]\n", > > + vp->id, win->data->name, actual_w, actual_h, dsp_w, dsp_h, > > + dest->x1, dest->y1, > > + &fb->format->format, > > + afbc_en ? "AFBC" : "", &yrgb_mst); > > + > > + if (afbc_en) { > > + uint32_t stride; > > + > > + /* the afbc superblock is 16 x 16 */ > > + afbc_format = vop2_convert_afbc_format(fb->format->format); > > + > > + /* Enable color transform for YTR */ > > + if (fb->modifier & AFBC_FORMAT_MOD_YTR) > > + afbc_format |= (1 << 4); > > + > > + afbc_tile_num = ALIGN(actual_w, 16) >> 4; > > + > > + /* > > + * AFBC pic_vir_width is count by pixel, this is different > > + * with WIN_VIR_STRIDE. > > + */ > > + stride = (fb->pitches[0] << 3) / bpp; > > + if ((stride & 0x3f) && (xmirror || rotate_90 || rotate_270)) > > + drm_err(vop2->drm, "vp%d %s stride[%d] must align as 64 pixel when enable xmirror/rotate_90/rotate_270[0x%x]\n", > > + vp->id, win->data->name, stride, pstate->rotation); > > + > > + rb_swap = vop2_afbc_rb_swap(fb->format->format); > > + uv_swap = vop2_afbc_uv_swap(fb->format->format); > > + /* > > + * This is a workaround for crazy IC design, Cluster > > + * and Esmart/Smart use different format configuration map: > > + * YUV420_10BIT: 0x10 for Cluster, 0x14 for Esmart/Smart. > > + * > > + * This is one thing we can make the convert simple: > > + * AFBCD decode all the YUV data to YUV444. So we just > > + * set all the yuv 10 bit to YUV444_10. > > + */ > > + if (fb->format->is_yuv && (bpp == 10)) > > + format = VOP2_CLUSTER_YUV444_10; > > + > > + if (vop2_cluster_window(win)) > > + vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1); > > + vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format); > > + vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap); > > + vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap); > > + vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0); > > + vop2_win_write(win, VOP2_WIN_AFBC_BLOCK_SPLIT_EN, 0); > > + if (pstate->rotation & (DRM_MODE_ROTATE_270 | DRM_MODE_ROTATE_90)) { > > + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 0); > > + transform_offset = vop2_afbc_transform_offset(pstate, false); > > + } else { > > + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 1); > > + transform_offset = vop2_afbc_transform_offset(pstate, true); > > + } > > + vop2_win_write(win, VOP2_WIN_AFBC_HDR_PTR, yrgb_mst); > > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_SIZE, act_info); > > + vop2_win_write(win, VOP2_WIN_AFBC_TRANSFORM_OFFSET, transform_offset); > > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_OFFSET, ((src->x1 >> 16) | src->y1)); > > + vop2_win_write(win, VOP2_WIN_AFBC_DSP_OFFSET, (dest->x1 | (dest->y1 << 16))); > > + vop2_win_write(win, VOP2_WIN_AFBC_PIC_VIR_WIDTH, stride); > > + vop2_win_write(win, VOP2_WIN_AFBC_TILE_NUM, afbc_tile_num); > > + vop2_win_write(win, VOP2_WIN_XMIRROR, xmirror); > > The xmirror's variable type is specified as bool, but it's not > true/false because in the above code: > > bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; > > I don't see how vop2_win_write() could work properly. Or am I missing > something? Everything != 0 is true, so 0x10 & 0x10 is still true. But ok, bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X ? true : false; looks cleaner. I'll change that. Thanks for the review so far. Could you drop me a short note when you're done? I was about to send v6. Thanks Sascha
16.02.2022 14:22, Sascha Hauer пишет: > On Wed, Feb 16, 2022 at 01:39:33PM +0300, Dmitry Osipenko wrote: >> 09.02.2022 12:53, Sascha Hauer пишет: >>> +static void vop2_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) >>> +{ >>> + struct drm_plane_state *pstate = plane->state; >>> + struct drm_crtc *crtc = pstate->crtc; >>> + struct vop2_win *win = to_vop2_win(plane); >>> + struct vop2_video_port *vp = to_vop2_video_port(crtc); >>> + struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode; >>> + struct vop2 *vop2 = win->vop2; >>> + struct drm_framebuffer *fb = pstate->fb; >>> + uint32_t bpp = fb->format->cpp[0] * 8; >>> + uint32_t actual_w, actual_h, dsp_w, dsp_h; >>> + uint32_t act_info, dsp_info; >>> + uint32_t format; >>> + uint32_t afbc_format; >>> + uint32_t rb_swap; >>> + uint32_t uv_swap; >>> + struct drm_rect *src = &pstate->src; >>> + struct drm_rect *dest = &pstate->dst; >>> + uint32_t afbc_tile_num; >>> + uint32_t transform_offset; >>> + bool dither_up; >>> + bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; >>> + bool ymirror = pstate->rotation & DRM_MODE_REFLECT_Y; >>> + bool rotate_270 = pstate->rotation & DRM_MODE_ROTATE_270; >>> + bool rotate_90 = pstate->rotation & DRM_MODE_ROTATE_90; >>> + struct rockchip_gem_object *rk_obj; >>> + unsigned long offset; >>> + bool afbc_en; >>> + dma_addr_t yrgb_mst; >>> + dma_addr_t uv_mst; >>> + >>> + /* >>> + * can't update plane when vop2 is disabled. >>> + */ >>> + if (WARN_ON(!crtc)) >>> + return; >>> + >>> + if (!pstate->visible) { >>> + vop2_plane_atomic_disable(plane, state); >>> + return; >>> + } >>> + >>> + afbc_en = rockchip_afbc(plane, fb->modifier); >>> + >>> + offset = (src->x1 >> 16) * fb->format->cpp[0]; >>> + >>> + /* >>> + * AFBC HDR_PTR must set to the zero offset of the framebuffer. >>> + */ >>> + if (afbc_en) >>> + offset = 0; >>> + else if (pstate->rotation & DRM_MODE_REFLECT_Y) >>> + offset += ((src->y2 >> 16) - 1) * fb->pitches[0]; >>> + else >>> + offset += (src->y1 >> 16) * fb->pitches[0]; >>> + >>> + rk_obj = to_rockchip_obj(fb->obj[0]); >>> + >>> + yrgb_mst = rk_obj->dma_addr + offset + fb->offsets[0]; >>> + if (fb->format->is_yuv) { >>> + int hsub = fb->format->hsub; >>> + int vsub = fb->format->vsub; >>> + >>> + offset = (src->x1 >> 16) * fb->format->cpp[1] / hsub; >>> + offset += (src->y1 >> 16) * fb->pitches[1] / vsub; >>> + >>> + if ((pstate->rotation & DRM_MODE_REFLECT_Y) && !afbc_en) >>> + offset += fb->pitches[1] * ((pstate->src_h >> 16) - 2) / vsub; >>> + >>> + rk_obj = to_rockchip_obj(fb->obj[0]); >>> + uv_mst = rk_obj->dma_addr + offset + fb->offsets[1]; >>> + } >>> + >>> + actual_w = drm_rect_width(src) >> 16; >>> + actual_h = drm_rect_height(src) >> 16; >>> + dsp_w = drm_rect_width(dest); >>> + >>> + if (dest->x1 + dsp_w > adjusted_mode->hdisplay) { >>> + drm_err(vop2->drm, "vp%d %s dest->x1[%d] + dsp_w[%d] exceed mode hdisplay[%d]\n", >>> + vp->id, win->data->name, dest->x1, dsp_w, adjusted_mode->hdisplay); >>> + dsp_w = adjusted_mode->hdisplay - dest->x1; >>> + if (dsp_w < 4) >>> + dsp_w = 4; >>> + actual_w = dsp_w * actual_w / drm_rect_width(dest); >>> + } >>> + >>> + dsp_h = drm_rect_height(dest); >>> + >>> + if (dest->y1 + dsp_h > adjusted_mode->vdisplay) { >>> + drm_err(vop2->drm, "vp%d %s dest->y1[%d] + dsp_h[%d] exceed mode vdisplay[%d]\n", >>> + vp->id, win->data->name, dest->y1, dsp_h, adjusted_mode->vdisplay); >>> + dsp_h = adjusted_mode->vdisplay - dest->y1; >>> + if (dsp_h < 4) >>> + dsp_h = 4; >>> + actual_h = dsp_h * actual_h / drm_rect_height(dest); >>> + } >>> + >>> + /* >>> + * This is workaround solution for IC design: >>> + * esmart can't support scale down when actual_w % 16 == 1. >>> + */ >>> + if (!(win->data->feature & WIN_FEATURE_AFBDC)) { >>> + if (actual_w > dsp_w && (actual_w & 0xf) == 1) { >>> + drm_err(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n", vp->id, win->data->name, actual_w); >>> + actual_w -= 1; >>> + } >>> + } >>> + >>> + if (afbc_en && actual_w % 4) { >>> + drm_err(vop2->drm, "vp%d %s actual_w[%d] should align as 4 pixel when enable afbc\n", >>> + vp->id, win->data->name, actual_w); >>> + actual_w = ALIGN_DOWN(actual_w, 4); >>> + } >>> + >>> + act_info = (actual_h - 1) << 16 | ((actual_w - 1) & 0xffff); >>> + dsp_info = (dsp_h - 1) << 16 | ((dsp_w - 1) & 0xffff); >>> + >>> + format = vop2_convert_format(fb->format->format); >>> + >>> + drm_dbg(vop2->drm, "vp%d update %s[%dx%d->%dx%d@%dx%d] fmt[%p4cc_%s] addr[%pad]\n", >>> + vp->id, win->data->name, actual_w, actual_h, dsp_w, dsp_h, >>> + dest->x1, dest->y1, >>> + &fb->format->format, >>> + afbc_en ? "AFBC" : "", &yrgb_mst); >>> + >>> + if (afbc_en) { >>> + uint32_t stride; >>> + >>> + /* the afbc superblock is 16 x 16 */ >>> + afbc_format = vop2_convert_afbc_format(fb->format->format); >>> + >>> + /* Enable color transform for YTR */ >>> + if (fb->modifier & AFBC_FORMAT_MOD_YTR) >>> + afbc_format |= (1 << 4); >>> + >>> + afbc_tile_num = ALIGN(actual_w, 16) >> 4; >>> + >>> + /* >>> + * AFBC pic_vir_width is count by pixel, this is different >>> + * with WIN_VIR_STRIDE. >>> + */ >>> + stride = (fb->pitches[0] << 3) / bpp; >>> + if ((stride & 0x3f) && (xmirror || rotate_90 || rotate_270)) >>> + drm_err(vop2->drm, "vp%d %s stride[%d] must align as 64 pixel when enable xmirror/rotate_90/rotate_270[0x%x]\n", >>> + vp->id, win->data->name, stride, pstate->rotation); >>> + >>> + rb_swap = vop2_afbc_rb_swap(fb->format->format); >>> + uv_swap = vop2_afbc_uv_swap(fb->format->format); >>> + /* >>> + * This is a workaround for crazy IC design, Cluster >>> + * and Esmart/Smart use different format configuration map: >>> + * YUV420_10BIT: 0x10 for Cluster, 0x14 for Esmart/Smart. >>> + * >>> + * This is one thing we can make the convert simple: >>> + * AFBCD decode all the YUV data to YUV444. So we just >>> + * set all the yuv 10 bit to YUV444_10. >>> + */ >>> + if (fb->format->is_yuv && (bpp == 10)) >>> + format = VOP2_CLUSTER_YUV444_10; >>> + >>> + if (vop2_cluster_window(win)) >>> + vop2_win_write(win, VOP2_WIN_AFBC_ENABLE, 1); >>> + vop2_win_write(win, VOP2_WIN_AFBC_FORMAT, afbc_format); >>> + vop2_win_write(win, VOP2_WIN_AFBC_RB_SWAP, rb_swap); >>> + vop2_win_write(win, VOP2_WIN_AFBC_UV_SWAP, uv_swap); >>> + vop2_win_write(win, VOP2_WIN_AFBC_AUTO_GATING_EN, 0); >>> + vop2_win_write(win, VOP2_WIN_AFBC_BLOCK_SPLIT_EN, 0); >>> + if (pstate->rotation & (DRM_MODE_ROTATE_270 | DRM_MODE_ROTATE_90)) { >>> + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 0); >>> + transform_offset = vop2_afbc_transform_offset(pstate, false); >>> + } else { >>> + vop2_win_write(win, VOP2_WIN_AFBC_HALF_BLOCK_EN, 1); >>> + transform_offset = vop2_afbc_transform_offset(pstate, true); >>> + } >>> + vop2_win_write(win, VOP2_WIN_AFBC_HDR_PTR, yrgb_mst); >>> + vop2_win_write(win, VOP2_WIN_AFBC_PIC_SIZE, act_info); >>> + vop2_win_write(win, VOP2_WIN_AFBC_TRANSFORM_OFFSET, transform_offset); >>> + vop2_win_write(win, VOP2_WIN_AFBC_PIC_OFFSET, ((src->x1 >> 16) | src->y1)); >>> + vop2_win_write(win, VOP2_WIN_AFBC_DSP_OFFSET, (dest->x1 | (dest->y1 << 16))); >>> + vop2_win_write(win, VOP2_WIN_AFBC_PIC_VIR_WIDTH, stride); >>> + vop2_win_write(win, VOP2_WIN_AFBC_TILE_NUM, afbc_tile_num); >>> + vop2_win_write(win, VOP2_WIN_XMIRROR, xmirror); >> >> The xmirror's variable type is specified as bool, but it's not >> true/false because in the above code: >> >> bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X; >> >> I don't see how vop2_win_write() could work properly. Or am I missing >> something? > > Everything != 0 is true, so 0x10 & 0x10 is still true. Ah, my bad. I keep forgetting that kernel uses _Bool. > But ok, > > bool xmirror = pstate->rotation & DRM_MODE_REFLECT_X ? true : false; > > looks cleaner. I'll change that. > > Thanks for the review so far. Could you drop me a short note when you're > done? I was about to send v6. I don't have any more comments, please feel free to send v6.
This is v5 of adding RK356x VOP2 support. I've dropped the patches that Heiko already applied, for testing either apply to linux-next or pick the missing clk patches from v4. I consider this series being ready for primetime now. One problem might be patch 1 [drm/encoder: Add of_graph port to struct drm_encoder] to which I need an ack from Dave and/or Daniel I guess. I collected the Acks from Rob I got so far. [dt-bindings: display: rockchip: Add binding for VOP2] doesn't have an ack yet, but it should be ready now in v5. Sascha Changes since v4: - Reorder patches in a way that binding/dts/driver patches are closer together - Drop clk patches already applied by Heiko Changes since v3: - added changelog to each patch - Add 4k support to hdmi driver - rebase on v5.17-rc1 Changes since v2: - Add pin names to HDMI supply pin description - Add hclk support to HDMI driver - Dual license rockchip-vop2 binding, update binding - Add HDMI connector to board dts files - drop unnecessary gamma_lut registers from vop2 - Update dclk_vop[012] clock handling, no longer hacks needed - Complete regmap conversion Changes since v1: - drop all unnecessary waiting for frames within atomic modeset and plane update - Cluster subwin support removed - gamma support removed - unnecessary irq_lock removed - interrupt handling simplified - simplified zpos handling - drop is_alpha_support(), use fb->format->has_alpha instead - use devm_regulator_get() rather than devm_regulator_get_optional() for hdmi regulators - Use fixed number of planes per video port - Drop homegrown regmap code from vop2 driver (not complete yet) - Add separate include file for vop2 driver to not pollute the vop include Andy Yan (1): drm: rockchip: Add VOP2 driver Benjamin Gaignard (1): dt-bindings: display: rockchip: dw-hdmi: Add compatible for rk3568 HDMI Douglas Anderson (2): drm/rockchip: dw_hdmi: Use auto-generated tables drm/rockchip: dw_hdmi: Set cur_ctr to 0 always Michael Riesch (1): arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a Nickey Yang (1): drm/rockchip: dw_hdmi: add default 594Mhz clk for 4K@60hz Sascha Hauer (17): drm/encoder: Add of_graph port to struct drm_encoder drm/rockchip: dw_hdmi: Do not leave clock enabled in error case drm/rockchip: dw_hdmi: rename vpll clock to reference clock dt-bindings: display: rockchip: dw-hdmi: use "ref" as clock name arm64: dts: rockchip: rk3399: rename HDMI ref clock to 'ref' drm/rockchip: dw_hdmi: add rk3568 support drm/rockchip: dw_hdmi: add regulator support dt-bindings: display: rockchip: dw-hdmi: Add regulator support drm/rockchip: dw_hdmi: Add support for hclk dt-bindings: display: rockchip: dw-hdmi: Add additional clock drm/rockchip: dw_hdmi: drop mode_valid hook dt-bindings: display: rockchip: dw-hdmi: Make unwedge pinctrl optional arm64: dts: rockchip: rk356x: Add VOP2 nodes arm64: dts: rockchip: rk356x: Add HDMI nodes arm64: dts: rockchip: rk3568-evb: Enable VOP2 and hdmi drm/rockchip: Make VOP driver optional dt-bindings: display: rockchip: Add binding for VOP2 .../display/rockchip/rockchip,dw-hdmi.yaml | 29 +- .../display/rockchip/rockchip-vop2.yaml | 140 + arch/arm64/boot/dts/rockchip/rk3399.dtsi | 2 +- .../boot/dts/rockchip/rk3566-quartz64-a.dts | 48 + arch/arm64/boot/dts/rockchip/rk3566.dtsi | 4 + .../boot/dts/rockchip/rk3568-evb1-v10.dts | 48 + arch/arm64/boot/dts/rockchip/rk3568.dtsi | 4 + arch/arm64/boot/dts/rockchip/rk356x.dtsi | 86 + drivers/gpu/drm/rockchip/Kconfig | 14 + drivers/gpu/drm/rockchip/Makefile | 4 +- drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 293 +- drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 3 +- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 7 +- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 + drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 15 + drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2689 +++++++++++++++++ drivers/gpu/drm/rockchip/rockchip_drm_vop2.h | 480 +++ drivers/gpu/drm/rockchip/rockchip_vop2_reg.c | 285 ++ include/drm/drm_encoder.h | 2 + include/dt-bindings/soc/rockchip,vop2.h | 14 + 20 files changed, 4045 insertions(+), 124 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_vop2.h create mode 100644 drivers/gpu/drm/rockchip/rockchip_vop2_reg.c create mode 100644 include/dt-bindings/soc/rockchip,vop2.h