Hi Robert,
Sorry it took me so long to have a closer look at this patchset.
I will definitely merge part of it, but this particular patch actually breaks i.MX 7. I have vertical stripes on my display with this patch applied (using Weston with DRM backend). Not sure why this exactly happens, from what I understand this should only affect IP Version 4.
Some notes below:
On 2019-08-29 13:30, Robert Chiras wrote:
Besides the eLCDIF block, there is another IP block, used in the past for EPDC panels. Since the iMX.8mq doesn't have an EPDC connector, this block is not documented, but we can use it to do additional operations on the frame buffer.
Hm, but this block is part of the ELCDIF block, in terms of clock, power domain etc?
In this case, we can use the pigeon registers from this IP block in order to do horizontal crop on the frame buffer processed by the eLCDIF block.
Signed-off-by: Robert Chiras robert.chiras@nxp.com Tested-by: Guido Günther agx@sigxcpu.org
drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 79 ++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 + drivers/gpu/drm/mxsfb/mxsfb_regs.h | 16 ++++++++ 3 files changed, 92 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index a12f53d..317575e 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -15,6 +15,7 @@
#include <video/videomode.h>
+#include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> #include <drm/drm_fb_cma_helper.h> @@ -435,13 +436,66 @@ void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb) clk_disable_unprepare(mxsfb->clk_axi); }
+void mxsfb_set_fb_hcrop(struct mxsfb_drm_private *mxsfb, u32 src_w, u32 fb_w) +{
- u32 mask_cnt, htotal, hcount;
- u32 vdctrl2, vdctrl3, vdctrl4, transfer_count;
- u32 pigeon_12_0, pigeon_12_1, pigeon_12_2;
- if (src_w == fb_w) {
writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_0);writel(0x0, mxsfb->base + HW_EPDC_PIGEON_12_1);return;- }
- transfer_count = readl(mxsfb->base + LCDC_V4_TRANSFER_COUNT);
- hcount = TRANSFER_COUNT_GET_HCOUNT(transfer_count);
- transfer_count &= ~TRANSFER_COUNT_SET_HCOUNT(0xffff);
- transfer_count |= TRANSFER_COUNT_SET_HCOUNT(fb_w);
- writel(transfer_count, mxsfb->base + LCDC_V4_TRANSFER_COUNT);
- vdctrl2 = readl(mxsfb->base + LCDC_VDCTRL2);
- htotal = VDCTRL2_GET_HSYNC_PERIOD(vdctrl2);
- htotal += fb_w - hcount;
- vdctrl2 &= ~VDCTRL2_SET_HSYNC_PERIOD(0x3ffff);
- vdctrl2 |= VDCTRL2_SET_HSYNC_PERIOD(htotal);
- writel(vdctrl2, mxsfb->base + LCDC_VDCTRL2);
- vdctrl4 = readl(mxsfb->base + LCDC_VDCTRL4);
- vdctrl4 &= ~SET_DOTCLK_H_VALID_DATA_CNT(0x3ffff);
- vdctrl4 |= SET_DOTCLK_H_VALID_DATA_CNT(fb_w);
- writel(vdctrl4, mxsfb->base + LCDC_VDCTRL4);
- /* configure related pigeon registers */
- vdctrl3 = readl(mxsfb->base + LCDC_VDCTRL3);
- mask_cnt = GET_HOR_WAIT_CNT(vdctrl3) - 5;
- pigeon_12_0 = PIGEON_12_0_SET_STATE_MASK(0x24) |
PIGEON_12_0_SET_MASK_CNT(mask_cnt) |PIGEON_12_0_SET_MASK_CNT_SEL(0x6) |PIGEON_12_0_POL_ACTIVE_LOW |PIGEON_12_0_EN;- writel(pigeon_12_0, mxsfb->base + HW_EPDC_PIGEON_12_0);
- pigeon_12_1 = PIGEON_12_1_SET_CLR_CNT(src_w) |
PIGEON_12_1_SET_SET_CNT(0x0);- writel(pigeon_12_1, mxsfb->base + HW_EPDC_PIGEON_12_1);
- pigeon_12_2 = 0x0;
- writel(pigeon_12_2, mxsfb->base + HW_EPDC_PIGEON_12_2);
+}
void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, struct drm_plane_state *state) { struct drm_simple_display_pipe *pipe = &mxsfb->pipe; struct drm_crtc *crtc = &pipe->crtc;
- struct drm_plane_state *new_state = pipe->plane.state;
- struct drm_framebuffer *fb = pipe->plane.state->fb; struct drm_pending_vblank_event *event;
- dma_addr_t paddr;
- u32 fb_addr, src_off, src_w, stride, cpp = 0;
dma_addr_t seems to be the better type here, why change?
spin_lock_irq(&crtc->dev->event_lock); event = crtc->state->event; @@ -456,10 +510,27 @@ void mxsfb_plane_atomic_update(struct mxsfb_drm_private *mxsfb, } spin_unlock_irq(&crtc->dev->event_lock);
- paddr = mxsfb_get_fb_paddr(mxsfb);
- if (paddr) {
- if (!fb)
return;- fb_addr = mxsfb_get_fb_paddr(mxsfb);
- if (mxsfb->devdata->ipversion >= 4) {
cpp = fb->format->cpp[0];src_off = (new_state->src_y >> 16) * fb->pitches[0] +(new_state->src_x >> 16) * cpp;fb_addr += fb->offsets[0] + src_off;- }
- if (fb_addr) { clk_prepare_enable(mxsfb->clk_axi);
writel(paddr, mxsfb->base + mxsfb->devdata->next_buf);
writel(fb_addr, mxsfb->base + mxsfb->devdata->next_buf);- if (mxsfb->devdata->ipversion >= 4 &&
unlikely(drm_atomic_crtc_needs_modeset(new_state->crtc->state))) {stride = DIV_ROUND_UP(fb->pitches[0], cpp);src_w = new_state->src_w >> 16;mxsfb_set_fb_hcrop(mxsfb, src_w, stride);- }
} diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 888b520..06d3bf0 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -133,6 +133,7 @@ static int mxsfb_atomic_helper_check(struct drm_device *dev, if (old_bpp != new_bpp) new_state->mode_changed = true; }
Can you also drop this unrelated change?
-- Stefan
return ret; }
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h index 0f63ba1..df3279b 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -145,6 +145,22 @@ #define DEBUG0_HSYNC BIT(26) #define DEBUG0_VSYNC BIT(25)
+/* pigeon registers for crop */ +#define HW_EPDC_PIGEON_12_0 0xb00 +#define HW_EPDC_PIGEON_12_1 0xb10 +#define HW_EPDC_PIGEON_12_2 0xb20
+#define PIGEON_12_0_SET_STATE_MASK(x) REG_PUT((x), 31, 24) +#define PIGEON_12_0_SET_MASK_CNT(x) REG_PUT((x), 23, 12) +#define PIGEON_12_0_SET_MASK_CNT_SEL(x) REG_PUT((x), 11, 8) +#define PIGEON_12_0_SET_OFFSET(x) REG_PUT((x), 7, 4) +#define PIGEON_12_0_SET_INC_SEL(x) REG_PUT((x), 3, 2) +#define PIGEON_12_0_POL_ACTIVE_LOW BIT(1) +#define PIGEON_12_0_EN BIT(0)
+#define PIGEON_12_1_SET_CLR_CNT(x) REG_PUT((x), 31, 16) +#define PIGEON_12_1_SET_SET_CNT(x) REG_PUT((x), 15, 0)
#define MXSFB_MIN_XRES 120 #define MXSFB_MIN_YRES 120 #define MXSFB_MAX_XRES 0xffff