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 linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html