Re: [PATCH 12/23] drm/exynos: Split manager/display/subdrv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/10/15 Inki Dae <inki.dae@xxxxxxxxxxx>:
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> index c417c90..ba63c72 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> @@ -26,24 +26,23 @@
>>   * exynos specific encoder structure.
>>   *
>>   * @drm_encoder: encoder object.
>> - * @manager: specific encoder has its own manager to control a hardware
>> - *     appropriately and we can access a hardware drawing on this manager.
>> + * @display: the display structure that maps to this encoder
>>   */
>>  struct exynos_drm_encoder {
>>         struct drm_crtc                 *old_crtc;
>>         struct drm_encoder              drm_encoder;
>> -       struct exynos_drm_manager       *manager;
>> +       struct exynos_drm_display       *display;
>>  };
>>
>>  static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
>>  {
>> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
>> -       struct exynos_drm_display_ops *display_ops = manager->display_ops;
>> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
>> +       struct exynos_drm_display *display = exynos_encoder->display;
>>
>>         DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
>>
>> -       if (display_ops && display_ops->dpms)
>> -               display_ops->dpms(manager->ctx, mode);
>> +       if (display->ops->dpms)
>> +               display->ops->dpms(display->ctx, mode);
>
> It's good to remove apply callback. However, it seems that this patch
> has a problem that dma channel of fimd isn't enabled after dpms goes
> from off to on. So can you implement win_enable callback of fimd, and
> add it to fimd_win_resume function? We should have implemented
> win_enable callback.
>

Plz, ignore the above comments, and see the below comments.


>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 838c47d..f3dc808 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -105,7 +105,6 @@ struct fimd_win_data {
>>  };
>>
>>  struct fimd_context {
>> -       struct exynos_drm_subdrv        subdrv;
>>         struct device                   *dev;
>>         struct drm_device               *drm_dev;
>>         int                             irq;
>> @@ -120,6 +119,7 @@ struct fimd_context {
>>         u32                             vidcon0;
>>         u32                             vidcon1;
>>         bool                            suspended;
>> +       int                             pipe;
>>         struct mutex                    lock;
>>         wait_queue_head_t               wait_vsync_queue;
>>         atomic_t                        wait_vsync_event;
>> @@ -169,12 +169,16 @@ static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
>>  }
>>
>>  static struct exynos_drm_display_ops fimd_display_ops = {
>> -       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>         .is_connected = fimd_display_is_connected,
>>         .get_panel = fimd_get_panel,
>>         .check_mode = fimd_check_mode,
>>  };
>>
>> +static struct exynos_drm_display fimd_display = {
>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>> +       .ops = &fimd_display_ops,
>> +};
>> +
>>  static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -481,15 +485,46 @@ static void fimd_win_disable(void *in_ctx, int zpos)
>>         win_data->enabled = false;
>>  }
>>
>> -static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
>> +static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
>> +               int pipe)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>>
>>         ctx->drm_dev = drm_dev;
>> +       ctx->pipe = pipe;
>> +
>> +       /*
>> +        * enable drm irq mode.
>> +        * - with irq_enabled = 1, we can use the vblank feature.
>> +        *
>> +        * P.S. note that we wouldn't use drm irq handler but
>> +        *      just specific driver own one instead because
>> +        *      drm framework supports only one irq handler.
>> +        */
>> +       ctx->drm_dev->irq_enabled = 1;
>> +
>> +       /*
>> +        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>> +        * by drm timer once a current process gives up ownership of
>> +        * vblank event.(after drm_vblank_put function is called)
>> +        */
>> +       drm_dev->vblank_disable_allowed = 1;
>> +
>> +       /* attach this sub driver to iommu mapping if supported. */
>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>> +               drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
>>
>>         return 0;
>>  }
>>
>> +static void fimd_mgr_remove(void *in_ctx)
>> +{
>> +       struct fimd_context *ctx = in_ctx;
>> +
>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>> +               drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>> +}
>> +
>>  static void fimd_dpms(void *in_ctx, int mode)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -523,24 +558,6 @@ static void fimd_dpms(void *in_ctx, int mode)
>>         mutex_unlock(&ctx->lock);
>>  }
>>
>> -static void fimd_apply(void *in_ctx)
>> -{
>> -       struct fimd_context *ctx = in_ctx;
>> -       struct exynos_drm_manager *mgr = ctx->subdrv.manager;
>> -       struct exynos_drm_manager_ops *mgr_ops = mgr->ops;
>> -       struct fimd_win_data *win_data;
>> -       int i;
>> -
>> -       for (i = 0; i < WINDOWS_NR; i++) {
>> -               win_data = &ctx->win_data[i];
>> -               if (win_data->enabled && (mgr_ops && mgr_ops->win_commit))
>> -                       mgr_ops->win_commit(ctx, i);
>> -       }
>> -
>> -       if (mgr_ops && mgr_ops->commit)
>> -               mgr_ops->commit(ctx);
>> -}
>> -
>>  static void fimd_commit(void *in_ctx)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -597,6 +614,21 @@ static void fimd_commit(void *in_ctx)
>>         writel(val, ctx->regs + VIDCON0);
>>  }
>>
>> +static void fimd_apply(void *in_ctx)
>> +{
>> +       struct fimd_context *ctx = in_ctx;
>> +       struct fimd_win_data *win_data;
>> +       int i;
>> +
>> +       for (i = 0; i < WINDOWS_NR; i++) {
>> +               win_data = &ctx->win_data[i];
>> +               if (win_data->enabled)
>> +                       fimd_win_commit(ctx, i);

Implement fimd_win_enable function and call it at here,

                      if (win_data->enabled) {
                             fimd_win_commit(ctx,i);
                             fimd_win_enable(ctx, i);  <- here
                      }

And, the below codes, for enable overlay dma channel, should be
removed from fimd_win_commit function,

    /* wincon */
    val = readl(ctx->regs + WINCON(win));
    val |= WINCONx_ENWIN;
    writel(val, ctx->regs + WINCON(win));


And, with [PATCH v2] drm/exynos: fimd: clean up pm suspend/resume, it
must work well.

>> +       }
>> +
>> +       fimd_commit(ctx);
>> +}
>> +
>>  static int fimd_enable_vblank(void *in_ctx)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -661,6 +693,7 @@ static void fimd_wait_for_vblank(void *in_ctx)
>>
>>  static struct exynos_drm_manager_ops fimd_manager_ops = {
>>         .initialize = fimd_mgr_initialize,
>> +       .remove = fimd_mgr_remove,
>>         .dpms = fimd_dpms,
>>         .apply = fimd_apply,
>>         .commit = fimd_commit,
>> @@ -673,16 +706,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
>>  };
>>
>>  static struct exynos_drm_manager fimd_manager = {
>> -       .pipe           = -1,
>> -       .ops            = &fimd_manager_ops,
>> -       .display_ops    = &fimd_display_ops,
>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>> +       .ops = &fimd_manager_ops,
>>  };
>>
>>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>  {
>>         struct fimd_context *ctx = (struct fimd_context *)dev_id;
>> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
>> -       struct exynos_drm_manager *manager = subdrv->manager;
>>         u32 val;
>>
>>         val = readl(ctx->regs + VIDINTCON1);
>> @@ -692,11 +722,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>                 writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
>>
>>         /* check the crtc is detached already from encoder */
>> -       if (manager->pipe < 0 || !ctx->drm_dev)
>> +       if (ctx->pipe < 0 || !ctx->drm_dev)
>>                 goto out;
>>
>> -       drm_handle_vblank(ctx->drm_dev, manager->pipe);
>> -       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
>> +       drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>> +       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>
>>         /* set wait vsync event to zero and wake up queue. */
>>         if (atomic_read(&ctx->wait_vsync_event)) {
>> @@ -707,39 +737,6 @@ out:
>>         return IRQ_HANDLED;
>>  }
>>
>> -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>> -{
>> -       /*
>> -        * enable drm irq mode.
>> -        * - with irq_enabled = 1, we can use the vblank feature.
>> -        *
>> -        * P.S. note that we wouldn't use drm irq handler but
>> -        *      just specific driver own one instead because
>> -        *      drm framework supports only one irq handler.
>> -        */
>> -       drm_dev->irq_enabled = 1;
>> -
>> -       /*
>> -        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>> -        * by drm timer once a current process gives up ownership of
>> -        * vblank event.(after drm_vblank_put function is called)
>> -        */
>> -       drm_dev->vblank_disable_allowed = 1;
>> -
>> -       /* attach this sub driver to iommu mapping if supported. */
>> -       if (is_drm_iommu_supported(drm_dev))
>> -               drm_iommu_attach_device(drm_dev, dev);
>> -
>> -       return 0;
>> -}
>> -
>> -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>> -{
>> -       /* detach this sub driver from iommu mapping if supported. */
>> -       if (is_drm_iommu_supported(drm_dev))
>> -               drm_iommu_detach_device(drm_dev, dev);
>> -}
>> -
>>  static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
>>  {
>>         struct videomode *vm = &ctx->panel.vm;
>> @@ -825,9 +822,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable)
>>         return 0;
>>  }
>>
>> -static void fimd_window_suspend(struct device *dev)
>> +static void fimd_window_suspend(struct fimd_context *ctx)
>>  {
>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>         struct fimd_win_data *win_data;
>>         int i;
>>
>> @@ -839,9 +835,8 @@ static void fimd_window_suspend(struct device *dev)
>>         fimd_wait_for_vblank(ctx);
>>  }
>>
>> -static void fimd_window_resume(struct device *dev)
>> +static void fimd_window_resume(struct fimd_context *ctx)
>>  {
>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>         struct fimd_win_data *win_data;
>>         int i;
>>
>> @@ -854,7 +849,6 @@ static void fimd_window_resume(struct device *dev)
>>
>>  static int fimd_activate(struct fimd_context *ctx, bool enable)
>>  {
>> -       struct device *dev = ctx->subdrv.dev;
>>         if (enable) {
>>                 int ret;
>>
>> @@ -866,11 +860,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable)
>>
>>                 /* if vblank was enabled status, enable it again. */
>>                 if (test_and_clear_bit(0, &ctx->irq_flags))
>> -                       fimd_enable_vblank(dev);
>> +                       fimd_enable_vblank(ctx);
>>
>> -               fimd_window_resume(dev);
>> +               fimd_window_resume(ctx);
>>         } else {
>> -               fimd_window_suspend(dev);
>> +               fimd_window_suspend(ctx);
>>
>>                 fimd_clock(ctx, false);
>>                 ctx->suspended = true;
>> @@ -907,7 +901,6 @@ static int fimd_probe(struct platform_device *pdev)
>>  {
>>         struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx;
>> -       struct exynos_drm_subdrv *subdrv;
>>         struct resource *res;
>>         int win;
>>         int ret = -EINVAL;
>> @@ -952,15 +945,6 @@ static int fimd_probe(struct platform_device *pdev)
>>         DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
>>         atomic_set(&ctx->wait_vsync_event, 0);
>>
>> -       fimd_manager.ctx = ctx;
>> -
>> -       subdrv = &ctx->subdrv;
>> -
>> -       subdrv->dev = dev;
>> -       subdrv->manager = &fimd_manager;
>> -       subdrv->probe = fimd_subdrv_probe;
>> -       subdrv->remove = fimd_subdrv_remove;
>> -
>>         mutex_init(&ctx->lock);
>>
>>         platform_set_drvdata(pdev, ctx);
>> @@ -971,7 +955,11 @@ static int fimd_probe(struct platform_device *pdev)
>>         for (win = 0; win < WINDOWS_NR; win++)
>>                 fimd_clear_win(ctx, win);
>>
>> -       exynos_drm_subdrv_register(subdrv);
>> +       fimd_manager.ctx = ctx;
>> +       exynos_drm_manager_register(&fimd_manager);
>> +
>> +       fimd_display.ctx = ctx;
>> +       exynos_drm_display_register(&fimd_display);
>>
>>         return 0;
>>  }
>> @@ -981,7 +969,8 @@ static int fimd_remove(struct platform_device *pdev)
>>         struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx = platform_get_drvdata(pdev);
>>
>> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
>> +       exynos_drm_display_unregister(&fimd_display);
>> +       exynos_drm_manager_unregister(&fimd_manager);
>>
>>         if (ctx->suspended)
>>                 goto out;
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux