lkml.org 
[lkml]   [2016]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 1/4] drm/rockchip: vop: export line flag function
    On Fri, Jul 1, 2016 at 5:19 AM, Yakir Yang <ykk@rock-chips.com> wrote:
    > VOP have integrated a hardware counter which indicate the exact display
    > line that vop is scanning. And if we're interested in a specific line,
    > we can set the line number to vop line_flag register, and then vop would
    > generate a line_flag interrupt for it.
    >
    > For example eDP PSR function is interested in the vertical blanking
    > period, then driver could set the line number to zero.
    >
    > This patch have exported a symbol that allow other driver to listen the
    > line flag event with given timeout limit:
    > - rockchip_drm_wait_line_flag()
    >
    > Signed-off-by: Yakir Yang <ykk@rock-chips.com>
    > ---
    > Changes in v3:
    > - Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
    > - Add 'line_flag_num_0' for RK3288/RK3036
    > - Remove the notify for waiting line_flag event (Daniel)
    >
    > Changes in v2:
    > - Introduce in v2, split VOP line flag changes out
    >
    > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 3 +
    > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 103 ++++++++++++++++++++++++++++
    > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 3 +
    > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 4 ++
    > 4 files changed, 113 insertions(+)
    >
    > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    > index ea39329..239b830 100644
    > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
    > @@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
    > struct device *dev);
    > void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
    > struct device *dev);
    > +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
    > + unsigned int mstimeout);
    > +
    > #endif /* _ROCKCHIP_DRM_DRV_H_ */
    > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    > index c8a62a8..cd3cac5 100644
    > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
    > @@ -121,6 +121,8 @@ struct vop {
    > /* protected by dev->event_lock */
    > struct drm_pending_vblank_event *event;
    >
    > + struct completion line_flag_completion;
    > +
    > const struct vop_data *data;
    >
    > uint32_t *regsbak;
    > @@ -431,6 +433,59 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
    > spin_unlock_irqrestore(&vop->irq_lock, flags);
    > }
    >
    > +/*
    > + * (1) each frame starts at the start of the Vsync pulse which is signaled by
    > + * the "FRAME_SYNC" interrupt.
    > + * (2) the active data region of each frame ends at dsp_vact_end
    > + * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
    > + * to get "LINE_FLAG" interrupt at the end of the active on screen data.
    > + *
    > + * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
    > + * Interrupts
    > + * LINE_FLAG -------------------------------+
    > + * FRAME_SYNC ----+ |
    > + * | |
    > + * v v
    > + * | Vsync | Vbp | Vactive | Vfp |
    > + * ^ ^ ^ ^
    > + * | | | |
    > + * | | | |
    > + * dsp_vs_end ------------+ | | | VOP_DSP_VTOTAL_VS_END
    > + * dsp_vact_start --------------+ | | VOP_DSP_VACT_ST_END
    > + * dsp_vact_end ----------------------------+ | VOP_DSP_VACT_ST_END
    > + * dsp_total -------------------------------------+ VOP_DSP_VTOTAL_VS_END
    > + */
    > +
    > +static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
    > +{
    > + unsigned long flags;
    > +
    > + if (WARN_ON(!vop->is_enabled))
    > + return;
    > +
    > + spin_lock_irqsave(&vop->irq_lock, flags);
    > +
    > + VOP_CTRL_SET(vop, line_flag_num_0, line_num);
    > + VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
    > + vop_cfg_done(vop);
    > +
    > + spin_unlock_irqrestore(&vop->irq_lock, flags);
    > +}
    > +
    > +static void vop_line_flag_irq_disable(struct vop *vop)
    > +{
    > + unsigned long flags;
    > +
    > + if (WARN_ON(!vop->is_enabled))
    > + return;
    > +
    > + spin_lock_irqsave(&vop->irq_lock, flags);
    > +
    > + VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
    > +
    > + spin_unlock_irqrestore(&vop->irq_lock, flags);
    > +}
    > +
    > static void vop_enable(struct drm_crtc *crtc)
    > {
    > struct vop *vop = to_vop(crtc);
    > @@ -1157,6 +1212,13 @@ static irqreturn_t vop_isr(int irq, void *data)
    > ret = IRQ_HANDLED;
    > }
    >
    > + if (active_irqs & LINE_FLAG_INTR) {
    > + if (!completion_done(&vop->line_flag_completion))
    > + complete(&vop->line_flag_completion);

    I think there's potential to miss flags here if the timing is just
    right because the completion_done and complete are not atomic wrt the
    wait_line_flag function. I think you need some locking around the
    completion operations to ensure they're properly sequenced.

    FWIW, if you're reinitializing the completion every time in
    drm_wait_line_flag, you should be able to just call complete (or
    complete_all) without checking if there are any waiters.


    > + active_irqs &= ~LINE_FLAG_INTR;
    > + ret = IRQ_HANDLED;
    > + }
    > +
    > if (active_irqs & FS_INTR) {
    > drm_crtc_handle_vblank(crtc);
    > vop_handle_vblank(vop);
    > @@ -1255,6 +1317,7 @@ static int vop_create_crtc(struct vop *vop)
    >
    > init_completion(&vop->dsp_hold_completion);
    > init_completion(&vop->wait_update_complete);
    > + init_completion(&vop->line_flag_completion);
    > crtc->port = port;
    > rockchip_register_crtc_funcs(crtc, &private_crtc_funcs);
    >
    > @@ -1411,6 +1474,46 @@ static void vop_win_init(struct vop *vop)
    > }
    > }
    >
    > +/**
    > + * rockchip_drm_wait_line_flag - acqiure the give line flag event
    > + * @crtc: CRTC to enable line flag
    > + * @line_num: interested line number
    > + * @mstimeout: millisecond for timeout
    > + *
    > + * Driver would hold here until the interested line flag interrupt have
    > + * happened or timeout to wait.
    > + *
    > + * Returns:
    > + * Zero on success, negative errno on failure.
    > + */
    > +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
    > + unsigned int mstimeout)
    > +{
    > + struct vop *vop = to_vop(crtc);
    > + unsigned long jiffies_left;
    > +
    > + if (!crtc || !vop->is_enabled)
    > + return -ENODEV;
    > +
    > + if (line_num > crtc->mode.vtotal || mstimeout <= 0)
    > + return -EINVAL;
    > +
    > + reinit_completion(&vop->line_flag_completion);
    > + vop_line_flag_irq_enable(vop, line_num);
    > +

    This will only work for one waiter per vop. Is it worth enforcing this
    explicitly to avoid weird behavior when there are more than one?

    > + jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
    > + msecs_to_jiffies(mstimeout));
    > + vop_line_flag_irq_disable(vop);
    > +
    > + if (jiffies_left == 0) {
    > + dev_err(vop->dev, "Timeout waiting for IRQ\n");
    > + return -ETIMEDOUT;
    > + }
    > +
    > + return 0;
    > +}
    > +EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
    > +
    > static int vop_bind(struct device *dev, struct device *master, void *data)
    > {
    > struct platform_device *pdev = to_platform_device(dev);
    > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
    > index ff4f52e..34fcd03 100644
    > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
    > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
    > @@ -61,6 +61,9 @@ struct vop_ctrl {
    > struct vop_reg hpost_st_end;
    > struct vop_reg vpost_st_end;
    >
    > + struct vop_reg line_flag_num_0;
    > + struct vop_reg line_flag_num_1;
    > +

    nit: you could make this an array:

    struct vop_reg line_flag_num[2];


    > struct vop_reg cfg_done;
    > };
    >
    > diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
    > index 6f42e56..e9211c9 100644
    > --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
    > +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
    > @@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
    > .hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
    > .vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
    > .vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
    > + .line_flag_num_0 = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
    > .cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
    > };
    >
    > @@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
    > .vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
    > .hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
    > .vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
    > + .line_flag_num_0 = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
    > .cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
    > };
    >
    > @@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
    > .vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
    > .hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
    > .vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
    > + .line_flag_num_0 = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
    > + .line_flag_num_1 = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
    > .cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
    > };
    >
    > --
    > 1.9.1
    >
    >

    \
     
     \ /
      Last update: 2016-07-01 18:01    [W:4.495 / U:0.168 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site