[PATCH V2] drm/exynos: Support DP CLKCON register in FIMD driver

[Ajay kumar]

Hi Inki,

Thanks for the review.

On Fri, Jun 27, 2014 at 1:04 PM, Inki Dae <inki.dae at samsung.com> wrote:
> On 2014년 06월 26일 23:36, Ajay Kumar wrote:
>> Add the missing setting for DP CLKCON register.
>>
>> This register is present on Exynos5 based FIMD controllers,
>> and needs to be used if we are using DP.
>>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>> ---
>> Changes since V1:
>>  - Remove usage of driver_data to configure DP CLKCON register
>>  drivers/gpu/drm/exynos/exynos_dp_core.c  | 2 ++
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h  | 8 ++++++++
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++
>>  include/video/samsung_fimd.h             | 4 ++++
>>  4 files changed, 19 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> index 2e77a15..d8868f3 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
>>
>>       platform_set_drvdata(pdev, &exynos_dp_display);
>>
>> +     exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP;
>> +
>>       return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display);
>>  }
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 36535f3..1089744 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -60,6 +60,12 @@ enum exynos_drm_output_type {
>>       EXYNOS_DISPLAY_TYPE_VIDI,
>>  };
>>
>> +enum exynos_fimd_output_type {
>> +     EXYNOS_FIMD_OUTPUT_MIPI,
>> +     EXYNOS_FIMD_OUTPUT_DPI,
>> +     EXYNOS_FIMD_OUTPUT_DP,
>> +};
>> +
>>  /*
>>   * Exynos drm common overlay structure.
>>   *
>> @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver;
>>  extern struct platform_driver rotator_driver;
>>  extern struct platform_driver gsc_driver;
>>  extern struct platform_driver ipp_driver;
>> +
>> +extern enum exynos_fimd_output_type exynos_fimd_output_type;
>>  #endif
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index bb45ab2..a46a9c4 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = {
>>       .has_shadowcon = 1,
>>  };
>>
>> +enum exynos_fimd_output_type exynos_fimd_output_type;
>> +
>>  struct fimd_win_data {
>>       unsigned int            offset_x;
>>       unsigned int            offset_y;
>> @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>>       if (clkdiv > 1)
>>               val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
>>
>> +     if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)
>> +             writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);
>
>
> You are passing output type directly from dp driver to fimd driver using
> extern variable. This is really not good. Is that really only way for it?
>
> One is,
> it's to add a dt property in fimd node and use it; ex.
> samsung,output_type. Whether fimd output should be DPI, DSI, or eDP
> would be dependent on Machine.
>
> Other is,
> crtc driver is to get output type though encoder callback. At this
> moment, crtc driver already knows encoder/connector driver. So I think
> you can use a callback of encoder to get output type like below,
>
> exynos_display_ops {
>         ...
>         ...get_output(struct drm_crtc *crtc);
> };
>
> at exynos_drm_encoder.c,
> ... exynos_drm_get_output_type(struct drm_crtc *crtc)
> {
>         ...
>         return display_ops->get_output(...);
> }
>
> at fimd_commit function,
>         ...
>         ctx->output_type = exynos_drm_get_output_type(mgr->crtc);
>
>
> I think first way is more simple and reasonable.
>
> One more thing, does FIMD ip of Exynos4 series have DP_CLKCON register?
> No, they have no any DP_CLKCON register so you would need to consider
> other SoC without DP_CLKCON. For this, a proper volunteer would be
> fimd_driver_data.
Right, I should also consider putting DP CLKCON register inside driver_data.
And, I will use the DT property you mentioned.
Will send a patchset ASAP.

Regards,
Ajay Kumar

> Thanks,
> Inki Dae
>
>
>> +
>>       writel(val, ctx->regs + VIDCON0);
>>  }
>>
>> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
>> index b039320..d8f4b0b 100644
>> --- a/include/video/samsung_fimd.h
>> +++ b/include/video/samsung_fimd.h
>> @@ -435,6 +435,10 @@
>>  #define BLENDCON_NEW_8BIT_ALPHA_VALUE                (1 << 0)
>>  #define BLENDCON_NEW_4BIT_ALPHA_VALUE                (0 << 0)
>>
>> +/* Video clock enable for DP */
>> +#define DP_CLKCON                            0x27C
>> +#define DP_CLK_ENABLE                                0x2
>> +
>>  /* Notes on per-window bpp settings
>>   *
>>   * Value     Win0     Win1     Win2     Win3     Win 4
>>
>