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

2014-06-30 14:31 GMT+09:00 Andrzej Hajda [off-list ref]:

On 06/30/2014 03:14 AM, Jingoo Han wrote:

quoted

On Friday, June 27, 2014 10:03 PM, Ajay kumar wrote:

quoted

On Fri, Jun 27, 2014 at 6:14 PM, Andrzej Hajda [off-list ref] wrote:

quoted

On 06/27/2014 01:48 PM, Ajay kumar wrote:

quoted

On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda [off-list ref] wrote:

quoted

+CC DT

On 06/27/2014 12:12 PM, Ajay Kumar wrote:

quoted

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 <redacted>
---
 .../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(+)

[.....]

quoted

quoted

quoted

quoted

quoted

 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.

quoted

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.

quoted

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.

Right, and I cannot understand why mDNIe and DP clock enable bit
exists in FIMD ip. :(

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.

It wouldn't be best way only to avoid adding DT binding. DT binding
could be good way to handle complicated hardware pipelines if needed.
Of course, if driver can handle it simply, it would be better to avoid
adding DT binding. However, Exynos SoC are complicated.

Exynos SoC have more IPs to should be considered; SMIES, mDNIe and MIE
as image enhancement devices, and eDP, MIPI-DSI, and DPI (FIMD
connected to panel directly) as Display bus devices and parallel panel
device. And image enhancement device and Display bus device can be
used together.

FIMD -------- Panel
FIMD -------- Display bus device -------- Panel
FIMD -------- image enhancement device -------- Panel
FIMD -------- image enhancement device -------- FIMD-Lite -------- Panel
FIMD -------- image enhancement device -------- Display bus device
-------- Panel
FIMD -------- image enhancement device -------- FIMD-Lite --------
Display bus device -------- Panel

And Display bus devices and parallel device couldn't be switched in
runtime since kernel has been booted. However, image enhancement
devices can be enabled or disabled in runtime so the output path of
FIMD can be changed to another path dynamically - actually, I had
handled such scenarios. So if Exynos drm driver should be considered
for above all cases, it'd make Eyxnos drm driver too complicated.

If DT people and other SoC maintainers give us your opinions, it would
be helpful for us. I will look into other SoC how they are handling
similar cases.

Thanks,
Inki Dae

Regards
Andrzej

quoted

Best regards,
Jingoo Han

quoted

Regards,
Ajay

[....]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel