[PATCH v2 06/10] drm/hisilicon: Add vblank feature
Xinliang Liu
xinliang.liu at linaro.org
Mon Nov 30 19:16:19 PST 2015
On 30 November 2015 at 15:54, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Sat, Nov 28, 2015 at 06:39:01PM +0800, Xinliang Liu wrote:
>> Add vblank handle for ADE.
>>
>> Signed-off-by: Xinliang Liu <xinliang.liu at linaro.org>
>> Signed-off-by: Xinwei Kong <kong.kongxinwei at hisilicon.com>
>> Signed-off-by: Andy Green <andy.green at linaro.org>
>> ---
>> drivers/gpu/drm/hisilicon/hisi_drm_ade.c | 78 ++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/hisilicon/hisi_drm_ade.h | 16 +++++++
>> drivers/gpu/drm/hisilicon/hisi_drm_drv.c | 19 +++++++-
>> 3 files changed, 112 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/gpu/drm/hisilicon/hisi_drm_ade.h
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hisi_drm_ade.c b/drivers/gpu/drm/hisilicon/hisi_drm_ade.c
>> index b0976c3..acb11e7 100644
>> --- a/drivers/gpu/drm/hisilicon/hisi_drm_ade.c
>> +++ b/drivers/gpu/drm/hisilicon/hisi_drm_ade.c
>> @@ -267,7 +267,79 @@ static void ade_power_down(struct ade_hw_ctx *ctx)
>> ctx->power_on = false;
>> }
>>
>> +static struct drm_crtc *hisi_get_crtc_from_index(struct drm_device *dev,
>> + unsigned int index)
>
> Ugly that you had to add this, but unfortunately necessary :( Fixing up
> the drm vblank hooks so that they deal with struct drm_crtc directly is
> somewhere on my todo. But drm_irq.c is still a bit a mess, so this is some
> ways off still.
>
> What might be possible as a follow-up cleanup though is to add
> vblank_enable and vblank_disable functions to struct
> drm_crtc_helper_funcs. And then provide this code here to map from int
> index to struct drm_crtc * as helpers in a new drm_vblank_helper.c file.
> That might be a good intermediate step.
I would like to have a try to make this intermediate step patch and
send out for review soon.
> But nothing that needs to be done before merging hisilicon, that's for
> sure.
>
> One more comment below.
>
>> +{
>> + unsigned int index_tmp = 0;
>> + struct drm_crtc *crtc;
>> +
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>> + if (index_tmp == index)
>> + return crtc;
>> +
>> + index_tmp++;
>> + }
>> +
>> + WARN_ON(true);
>> +}
>> +
>> +int ade_enable_vblank(struct drm_device *dev, int crtc_index)
>> +{
>> + struct drm_crtc *crtc = hisi_get_crtc_from_index(dev, crtc_index);
>> + struct ade_crtc *acrtc = to_ade_crtc(crtc);
>> + struct ade_hw_ctx *ctx = acrtc->ctx;
>> + void __iomem *base = ctx->base;
>> + u32 intr_en;
>> +
>> + DRM_INFO("enable_vblank enter.\n");
>> + if (!ctx->power_on)
>> + (void)ade_power_up(ctx);
>> +
>> + intr_en = readl(base + LDI_INT_EN);
>> + intr_en |= LDI_ISR_FRAME_END_INT;
>> + writel(intr_en, base + LDI_INT_EN);
>>
>> + return 0;
>> +}
>> +
>> +void ade_disable_vblank(struct drm_device *dev, int crtc_index)
>> +{
>> + struct drm_crtc *crtc = hisi_get_crtc_from_index(dev, crtc_index);
>> + struct ade_crtc *acrtc = to_ade_crtc(crtc);
>> + struct ade_hw_ctx *ctx = acrtc->ctx;
>> + void __iomem *base = ctx->base;
>> + u32 intr_en;
>> +
>> + DRM_INFO("disable_vblank enter.\n");
>> + if (!ctx->power_on) {
>> + DRM_ERROR("power is down! vblank disable fail\n");
>> + return;
>> + }
>> + intr_en = readl(base + LDI_INT_EN);
>> + intr_en &= ~LDI_ISR_FRAME_END_INT;
>> + writel(intr_en, base + LDI_INT_EN);
>> +}
>> +
>> +static irqreturn_t ade_irq_handler(int irq, void *data)
>> +{
>> + struct ade_crtc *acrtc = data;
>> + struct ade_hw_ctx *ctx = acrtc->ctx;
>> + struct drm_crtc *crtc = &acrtc->base;
>> + struct drm_device *dev = crtc->dev;
>> + void __iomem *base = ctx->base;
>> + u32 status;
>> +
>> + status = readl(base + LDI_MSK_INT);
>> + /* DRM_INFO("LDI IRQ: status=0x%X\n",status); */
>> +
>> + /* vblank irq */
>> + if (status & LDI_ISR_FRAME_END_INT) {
>> + writel(LDI_ISR_FRAME_END_INT, base + LDI_INT_CLR);
>> + drm_handle_vblank(dev, drm_crtc_index(crtc));
>
> drm_crtc_handle_vblank please. At least when calling into the vblank code
> drivers don't have to do the struct drm_crtc -> int index conversion any
> more. Please make sure you do that everywhere, in case I've missed one.
will use drm_crtc_handle_vblank in v3.
Thanks,
-xinliang
>
> Cheers, Daniel
>
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>>
>> /*
>> * set modules' reset mode: by software or hardware
>> @@ -858,6 +930,12 @@ static int ade_bind(struct device *dev, struct device *master, void *data)
>> if (ret)
>> return ret;
>>
>> + /* vblank irq init */
>> + ret = request_irq(ctx->irq, ade_irq_handler, DRIVER_IRQ_SHARED,
>> + drm_dev->driver->name, acrtc);
>> + if (ret)
>> + return ret;
>> +
>> return 0;
>> }
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hisi_drm_ade.h b/drivers/gpu/drm/hisilicon/hisi_drm_ade.h
>> new file mode 100644
>> index 0000000..d1d7b5d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hisi_drm_ade.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * Copyright (c) 2014-2015 Hisilicon Limited.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#ifndef __HISI_DRM_ADE_H__
>> +#define __HISI_DRM_ADE_H__
>> +
>> +int ade_enable_vblank(struct drm_device *dev, int crtc_index);
>> +void ade_disable_vblank(struct drm_device *dev, int crtc_index);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/hisilicon/hisi_drm_drv.c b/drivers/gpu/drm/hisilicon/hisi_drm_drv.c
>> index d0eca80..13f59aa 100644
>> --- a/drivers/gpu/drm/hisilicon/hisi_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hisi_drm_drv.c
>> @@ -21,6 +21,7 @@
>> #include <drm/drm_fb_cma_helper.h>
>> #include <drm/drm_atomic_helper.h>
>>
>> +#include "hisi_drm_ade.h"
>> #include "hisi_drm_drv.h"
>>
>> #define DRIVER_NAME "hisi-drm"
>> @@ -29,6 +30,7 @@ static int hisi_drm_unload(struct drm_device *dev)
>> {
>> struct hisi_drm_private *priv = dev->dev_private;
>>
>> + drm_vblank_cleanup(dev);
>> drm_mode_config_cleanup(dev);
>> devm_kfree(dev->dev, priv);
>> dev->dev_private = NULL;
>> @@ -76,11 +78,22 @@ static int hisi_drm_load(struct drm_device *dev, unsigned long flags)
>> goto err_mode_config_cleanup;
>> }
>>
>> + /* vblank init */
>> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
>> + if (ret) {
>> + DRM_ERROR("failed to initialize vblank.\n");
>> + goto err_unbind_all;
>> + }
>> + /* with irq_enabled = true, we can use the vblank feature. */
>> + dev->irq_enabled = true;
>> +
>> /* reset all the states of crtc/plane/encoder/connector */
>> drm_mode_config_reset(dev);
>>
>> return 0;
>>
>> +err_unbind_all:
>> + component_unbind_all(dev->dev, dev);
>> err_mode_config_cleanup:
>> drm_mode_config_cleanup(dev);
>> devm_kfree(dev->dev, priv);
>> @@ -126,7 +139,7 @@ static int hisi_gem_cma_dumb_create(struct drm_file *file,
>>
>> static struct drm_driver hisi_drm_driver = {
>> .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>> - DRIVER_ATOMIC,
>> + DRIVER_ATOMIC | DRIVER_HAVE_IRQ,
>> .load = hisi_drm_load,
>> .unload = hisi_drm_unload,
>> .fops = &hisi_drm_fops,
>> @@ -148,6 +161,10 @@ static struct drm_driver hisi_drm_driver = {
>> .gem_prime_vunmap = drm_gem_cma_prime_vunmap,
>> .gem_prime_mmap = drm_gem_cma_prime_mmap,
>>
>> + .get_vblank_counter = drm_vblank_count,
>> + .enable_vblank = ade_enable_vblank,
>> + .disable_vblank = ade_disable_vblank,
>> +
>> .name = "hisi",
>> .desc = "Hisilicon SoCs' DRM Driver",
>> .date = "20150718",
>> --
>> 1.9.1
>>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
More information about the dri-devel
mailing list