Hi Inki,
On Mon, Jun 30, 2014 at 11:01 AM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxx> wrote:
>>>> On 06/27/2014 01:48 PM, Ajay kumar wrote:
>>>>> On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>>>>>>> ---
>>>>>>> .../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.
Which approach do you think is better?
I shall make the patch for the same!
Thanks and regards,
Ajay Kumar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
