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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrej,

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:
>> Hi Andrej,
>>
>> On Fri, Jun 27, 2014 at 4:52 PM, Andrzej Hajda <a.hajda@xxxxxxxxxxx> wrote:
>>> Hi Ajay,
>>>
>>> +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(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt
>>>> index 2dad41b..12f3d7a 100644
>>>> --- a/Documentation/devicetree/bindings/video/samsung-fimd.txt
>>>> +++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt
>>>> @@ -41,6 +41,7 @@ Optional Properties:
>>>>  - samsung,power-domain: a phandle to FIMD power domain node.
>>>>  - samsung,invert-vden: video enable signal is inverted
>>>>  - samsung,invert-vclk: video clock signal is inverted
>>>> +- samsung,output-type: Type of display output interface(DPI=0, DSI=1, DP=2)
>>> There is no point in introducing this property. Exynos DRM have already
>>> logic which creates pipeline: fimd --> DPI|DSI|DP, this logic can be
>>> reused to determine display type. It can be done even without any
>>> additional callbacks, just by checking if there is connector of
>>> DRM_MODE_CONNECTOR_eDP type connected to fimd.
>> The mapping between crtc(struct exynos_drm_manager) and encoder(struct
>> exynos_drm_display)
>> in exynos drm happens by matching the exynos_drm_output_type variable in each
>> structure.
>> exynos_drm_output_type supports 3 types: LCD, HDMI and VIDI.
>> FIMD statically chooses EXYNOS_DISPLAY_TYPE_LCD as the output type,
>> and both DP and MIPI statically choose the same enum EXYNOS_DISPLAY_TYPE_LCD,
>> as output type.
>> So, we cannot use that logic to differentiate between DP/MIPI DSI.
>>
>> Also, checking based on connector type doesn't hold good.
>> The connector type will be DRM_MODE_CONNECTOR_LVDS in case of
>> DP->LVDS or MIPI->LVDS panels!
>
> True, I forgot about bridges. So additional callback/field is necessary.
> See below.
>
>>
>> Thanks and regards,
>> Ajay Kumar
>>
>>> Regards
>>> Andrzej
>>>
>>>>  - display-timings: timing settings for FIMD, as described in document [1].
>>>>               Can be used in case timings cannot be provided otherwise
>>>>               or to override timings provided by the panel.
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> index 33161ad..aa74e90 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> @@ -72,6 +72,7 @@ struct fimd_driver_data {
>>>>       unsigned int has_shadowcon:1;
>>>>       unsigned int has_clksel:1;
>>>>       unsigned int has_limited_fmt:1;
>>>> +     unsigned int has_dp_clkcon:1;
>>>>  };
>>>>
>>>>  static struct fimd_driver_data s3c64xx_fimd_driver_data = {
>>>> @@ -88,6 +89,13 @@ static struct fimd_driver_data exynos4_fimd_driver_data = {
>>>>  static struct fimd_driver_data exynos5_fimd_driver_data = {
>>>>       .timing_base = 0x20000,
>>>>       .has_shadowcon = 1,
>>>> +     .has_dp_clkcon = 1,
>>>> +};
>>>> +
>>>> +enum exynos_fimd_output_type {
>>>> +     EXYNOS_FIMD_OUTPUT_DPI,
>>>> +     EXYNOS_FIMD_OUTPUT_DSI,
>>>> +     EXYNOS_FIMD_OUTPUT_DP,
>>>>  };
>>>>
>>>>  struct fimd_win_data {
>>>> @@ -125,6 +133,8 @@ struct fimd_context {
>>>>       struct exynos_drm_panel_info panel;
>>>>       struct fimd_driver_data *driver_data;
>>>>       struct exynos_drm_display *display;
>>>> +
>>>> +     enum exynos_fimd_output_type exynos_fimd_output_type;
>>>>  };
>>>>
>>>>  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?

Regards,
Ajay


>>>>  }
>>>>
>>>> @@ -924,6 +938,7 @@ static int fimd_probe(struct platform_device *pdev)
>>>>       struct device *dev = &pdev->dev;
>>>>       struct fimd_context *ctx;
>>>>       struct resource *res;
>>>> +     u32 fimd_output_type;
>>>>       int ret = -EINVAL;
>>>>
>>>>       ret = exynos_drm_component_add(&pdev->dev, EXYNOS_DEVICE_TYPE_CRTC,
>>>> @@ -949,6 +964,14 @@ static int fimd_probe(struct platform_device *pdev)
>>>>               ctx->vidcon1 |= VIDCON1_INV_VDEN;
>>>>       if (of_property_read_bool(dev->of_node, "samsung,invert-vclk"))
>>>>               ctx->vidcon1 |= VIDCON1_INV_VCLK;
>>>> +     if (!of_property_read_u32(dev->of_node, "samsung,output-type",
>>>> +                             &fimd_output_type)) {
>>>> +             if ((fimd_output_type < EXYNOS_FIMD_OUTPUT_DPI) ||
>>>> +                 (fimd_output_type > EXYNOS_FIMD_OUTPUT_DP))
>>>> +                     dev_err(dev, "invalid output type for FIMD\n");
>>>> +             else
>>>> +                     ctx->exynos_fimd_output_type = fimd_output_type;
>>>> +     }
>>>>
>>>>       ctx->bus_clk = devm_clk_get(dev, "fimd");
>>>>       if (IS_ERR(ctx->bus_clk)) {
>>>> 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
>>>>
>
--
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




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux