[PATCH V3 1/7] drm/exynos: Support DP CLKCON register in FIMD driver

[Andrzej Hajda]

On 06/30/2014 03:14 AM, Jingoo Han wrote:
> On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:
>> On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda <a.hajda at samsung.com> wrote:
>>> On 06/27/2014 01:48 PM, Ajay kumar wrote:
>>>> On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda <a.hajda at samsung.com> wrote:
>>>>> +CC DT
>>>>>
>>>>> On 06/27/2014 12:12 PM, Ajay Kumar wrote:
>>>>>> Add the missing setting for DP CLKCON register.
>>>>>>
>>>>>> This register is present on Exynos5 based FIMD controllers,
>>>>>> and needs to be set if we are using DP.
>>>>>>
>>>>>> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>>>>>> ---
>>>>>>  .../devicetree/bindings/video/samsung-fimd.txt     |    1 +
>>>>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c           |   23 ++++++++++++++++++++
>>>>>>  include/video/samsung_fimd.h                       |    4 ++++
>>>>>>  3 files changed, 28 insertions(+)
> [.....]
>
>>>>>>  static const struct of_device_id fimd_driver_dt_match[] = {
>>>>>> @@ -331,6 +341,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>>>>>>       if (clkdiv > 1)
>>>>>>               val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR;
>>>>>>
>>>>>> +     if (ctx->driver_data->has_dp_clkcon &&
>>>>>> +             ctx->exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP)
>>>>>> +             writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON);
>>>>>> +
>>>>>>       writel(val, ctx->regs + VIDCON0);
>>> New code should not split VIDCON0 related code.It should be moved few
>>> lines above or few lines below.
>> Ok, for better readability.
>>
>>> Anyway this code should be rather placed in power related functions of
>>> dp encoder, as it enables dp. The only question
>>> is if DP_CLKCON update can be performed after VIDCON0 update. If yes the
>>> solution of the whole problem
>> I will check this.
>>
>>> seems to be simple:
>>> - fimd should provide function fimd_set_dp_clk_gate or sth similar,
>>> - this function should be called in exynos_dp_poweron/exynos_dp_poweroff.
>>> I hope I have not missed anything this time.
>> But, it won't look good to export a FIMD function which sets a FIMD register,
>> and call it in DP driver!
>> What does Inki/Jingoo have to say about this?
> I agree with Ajay Kumar's opinion.
> It doesn't look good to export the function to set FIMD register
> and call it by DP driver.

DP_CLKCON HW register shows clearly there is direct hardware dependency
between DP and FIMD.
Reflecting this dependency in drivers is just a consequence of HW design.
Moreover the register gates also clock for MDNIE, this solution can be
used there as well.

Anyway the most important is that we should avoid adding DT bindings for
things we can evaluate in drivers.

Regards
Andrzej

>
> Best regards,
> Jingoo Han
>> Regards,
>> Ajay
>>
> [....]
>
>