public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: k.kozlowski@samsung•com (Krzysztof Kozlowski)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v5 05/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range
Date: Wed, 30 Sep 2015 17:26:18 +0900	[thread overview]
Message-ID: <560B9CAA.9080109@samsung.com> (raw)
In-Reply-To: <560B9B33.2060409@rock-chips.com>

On 30.09.2015 17:20, Yakir Yang wrote:
> Hi Krzysztof,
> 
> On 09/30/2015 03:34 PM, Krzysztof Kozlowski wrote:
>> On 30.09.2015 16:19, Yakir Yang wrote:
>>> Hi Krzysztof,
>>>
>>> On 09/30/2015 01:32 PM, Krzysztof Kozlowski wrote:
>>>> On 22.09.2015 16:37, Yakir Yang wrote:
>>>>> Both hsync/vsync polarity and interlace mode can be parsed from
>>>>> drm display mode, and dynamic_range and ycbcr_coeff can be judge
>>>>> by the video code.
>>>>>
>>>>> But presumably Exynos still relies on the DT properties, so take
>>>>> good use of mode_fixup() in to achieve the compatibility hacks.
>>>>>
>>>>> Signed-off-by: Yakir Yang <ykk@rock-chips•com>
>>>>> ---
>>>>> Changes in v5:
>>>>> - Switch video timing type to "u32", so driver could use "of_property_read_u32"
>>>>>   to get the backword timing values. 
>>>> Okay
>>>>
>>>>> Krzysztof suggest me that driver could use
>>>>>   the "of_property_read_bool" to get backword timing values, but that interfacs
>>>>>   would modify the original drm_display_mode timing directly (whether those
>>>>>   properties exists or not).
>>>> Hmm, I don't understand. You have a:
>>>> 	struct video_info {
>>>> 		bool h_sync_polarity;
>>>> 		bool v_sync_polarity;
>>>> 		bool interlaced;
>>>> 	};
>>>>
>>>> so what is wrong with:
>>>> 	dp_video_config->h_sync_polarity =
>>>> 		of_property_read_bool(dp_node, "hsync-active-high");
>>>>
>>>> Is it exactly the same binding as previously?
>>> Yes, it is the same binding as previously. But just a note that we already
>>> mark those DT binding as deprecated.
>>>
>>> +-interlaced:            deprecated prop that can parsed frm drm_display_mode.
>>> +-vsync-active-high:     deprecated prop that can parsed frm drm_display_mode.
>>> +-hsync-active-high:     deprecated prop that can parsed frm drm_display_mode.
>>>
>>>
>>> For now those values should come from "struct drm_display_mode",
>>> and we already parsed them out from "drm_display_mode" before
>>> driver provide the backward compatibility.
>>>
>>> Let's used the "hsync-active-high" example:
>>>     As for now the code would like:
>>>     static void analogix_dp_bridge_mode_set(...)
>>>     {
>>>         // Parsed timing value from "drm_display_mode"
>>>         video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
>>>
>>>         // Try to detect the deprecated property, providing
>>>         // the backward compatibility
>>>         of_property_read_u32(dp_node, "hsync-active-high",
>>>                                  &video->h_sync_polarity);    
>>>
>>>         /*
>>>          * In this case, if "hsync-active-high" property haven't been
>>>          * found, then the video timing "h_sync_polarity" would  keep
>>>          * no change, keeping the parsed value from "drm_display_mode"
>>>          */     
>>>     }   
>>>
>>>     But if keep the "of_property_read_bool", then code would like:
>>>     static void analogix_dp_bridge_mode_set(...)
>>>     {
>>>         // Parsed timing value from "drm_display_mode"
>>>         video->h_sync_polarity = !!(mode->flags & DRM_MODE_FLAG_NHSYNC);
>>>
>>>         // Try to detect the deprecated property, providing
>>>         // the backward compatibility
>>>         video->h_sync_polarity =
>>>                     of_property_read_bool(dp_node, "hsync-active-high");
>>>    
>>>
>>>         /*
>>>          * In this case, if "hsync-active-high" property haven't been
>>>          * found, then the video timing "h_sync_polarity" would just
>>>          * modify to "false". That is the place we don't want, cause
>>>          * it would always modify the timing value parsed from
>>>          * "drm_display_mode"
>>>          */  
>>>     }   
>>>
>> OK, I see the point of overwriting values from drm_display_mode. However
>> I think you changed the binding. I believe the of_property_read_u32()
>> will behave differently for such DTS:
>>
>> exynos_dp {
>> 	...
>> 	hsync-active-high;
>> }
>>
>> It will return -EOVERFLOW which means it would be broken now...
> 
> Whoops, thanks for your remind, after try that, I do see over flow error.
> static void *of_find_property_value_of_size(const struct device_node *np,
>                         const char *propname, u32 len)
> {
>         ....
>         if (len > prop->length)
>                 return ERR_PTR(-EOVERFLOW);
>         ...
> }
> 
> So I though code should be:
>     if (of_property_read_bool(dp_node, "hsync-active-high"))
>         video->h_sync_polarity = true;

Looks good.

> 
> And we can't provide full backward compatibility for this property, cause
> the previous exynos_dp driver would set this timing value to "false" when
> property not defined, but analogix_dp driver keep this timing value
> corresponding to "drm_display_mode" when property not found.

Indeed, the behaviour changes. I don't know if this is important issue...

Best regards,
Krzysztof

  parent reply	other threads:[~2015-09-30  8:26 UTC|newest]

Thread overview: 133+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-01  5:46 [PATCH v4 0/16] Add Analogix Core Display Port Driver Yakir Yang
2015-09-01  5:46 ` [PATCH v4 01/16] drm: exynos/dp: fix code style Yakir Yang
2015-09-03  0:21   ` Krzysztof Kozlowski
2015-09-03  5:04     ` Yakir Yang
2015-09-03  5:08       ` Krzysztof Kozlowski
2015-09-03  5:33         ` Yakir Yang
2015-09-03  5:57           ` Joe Perches
2015-09-06  1:33             ` Yakir Yang
2015-09-01  5:49 ` [PATCH v4 02/16] drm: exynos/dp: convert to drm bridge mode Yakir Yang
2015-09-01  5:52 ` [PATCH v4 04/16] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang
2015-09-01  5:55 ` [PATCH v4 05/16] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & colorimetry Yakir Yang
2015-09-03  8:04   ` Krzysztof Kozlowski
2015-09-06  2:00     ` Yakir Yang
2015-09-01  5:58 ` [PATCH v4 06/16] Documentation: drm/bridge: add document for analogix_dp Yakir Yang
2015-09-01  6:01 ` [PATCH v4 07/16] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang
2015-09-03  0:01   ` Krzysztof Kozlowski
2015-09-01  6:01 ` [PATCH v4 08/16] drm: rockchip/dp: add rockchip platform dp driver Yakir Yang
2015-09-01 14:24   ` Heiko Stuebner
2015-09-01 21:00   ` Heiko Stuebner
2015-09-01  6:01 ` [PATCH v4 09/16] drm: rockchip: add bpc and color mode setting Yakir Yang
2015-09-01 21:00   ` Heiko Stuebner
2015-09-01  6:04 ` [PATCH v4 10/16] phy: Add driver for rockchip Display Port PHY Yakir Yang
2015-09-01 16:51   ` Heiko Stuebner
2015-09-01 20:58     ` Heiko Stuebner
2015-09-02 13:27   ` Rob Herring
2015-09-03  3:25     ` Yakir Yang
2015-09-03 13:52       ` Heiko Stuebner
2015-09-06  4:09         ` Yakir Yang
2015-09-01  6:07 ` [PATCH v4 11/16] drm: bridge: analogix/dp: add platform device type support Yakir Yang
2015-09-04  0:36   ` Krzysztof Kozlowski
2015-09-06  4:07     ` Yakir Yang
2015-09-06 23:55       ` Krzysztof Kozlowski
2015-09-01  6:09 ` [PATCH v4 12/16] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang
2015-09-01  6:11 ` [PATCH v4 13/16] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang
2015-09-01  6:14 ` [PATCH v4 14/16] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang
2015-09-02 20:17   ` Rob Herring
2015-09-03  4:27     ` Yakir Yang
2015-09-03  9:04       ` Thierry Reding
2015-09-04 10:20         ` Russell King - ARM Linux
2015-09-07  9:01           ` Thierry Reding
     [not found]         ` <55EBBA0C.1030100@rock-chips.com>
2015-09-07  8:20           ` Thierry Reding
2015-09-04 21:46       ` Rob Herring
2015-09-06  8:20         ` Yakir Yang
2015-09-07  8:39           ` Thierry Reding
2015-09-03  8:47     ` Thierry Reding
2015-09-03 21:55       ` Rob Herring
2015-09-04 10:01         ` Thierry Reding
2015-09-01  6:17 ` [PATCH v4 15/16] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang
2015-09-01  6:20 ` [PATCH v4 16/16] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang
     [not found] ` <1441086598-24995-1-git-send-email-ykk@rock-chips.com>
2015-09-01 20:46   ` [PATCH v4 03/16] drm: bridge: analogix/dp: split exynos dp driver to bridge dir Heiko Stuebner
2015-09-04 21:06     ` Rob Herring
2015-09-04 21:29       ` Heiko Stuebner
2015-09-07  8:11         ` Thierry Reding
2015-09-02 14:50   ` Emil Velikov
2015-09-03  3:55     ` Yakir Yang
2015-09-03  0:58   ` Krzysztof Kozlowski
2015-09-03  5:30     ` Yakir Yang
2015-09-04  0:41       ` Krzysztof Kozlowski
2015-09-06  7:49         ` Yakir Yang
2015-09-07  0:22           ` Krzysztof Kozlowski
2015-09-01 21:47 ` [PATCH v4 0/16] Add Analogix Core Display Port Driver Heiko Stuebner
2015-09-22  7:20 ` [PATCH v5 0/17] " Yakir Yang
2015-09-22  7:26   ` [PATCH v5 01/17] drm: exynos: dp: convert to drm bridge mode Yakir Yang
2015-09-22  7:34   ` [PATCH v5 03/17] drm: bridge: analogix/dp: fix some obvious code style Yakir Yang
2015-09-30  5:22     ` Krzysztof Kozlowski
2015-09-30  6:52       ` Yakir Yang
2015-09-22  7:35   ` [PATCH v5 04/17] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang
2015-09-22  7:37   ` [PATCH v5 05/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range Yakir Yang
2015-09-30  5:32     ` Krzysztof Kozlowski
     [not found]       ` <560B8CF1.7050102@rock-chips.com>
2015-09-30  7:34         ` Krzysztof Kozlowski
     [not found]           ` <560B9B33.2060409@rock-chips.com>
2015-09-30  8:26             ` Krzysztof Kozlowski [this message]
2015-09-30  9:39               ` Yakir Yang
2015-09-22  7:40   ` [PATCH v5 06/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang
2015-09-22  7:43   ` [PATCH v5 07/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang
2015-09-30  5:39     ` Krzysztof Kozlowski
2015-09-30  7:20       ` Yakir Yang
2015-09-22  7:45   ` [PATCH v5 08/17] drm: rockchip: dp: add rockchip platform dp driver Yakir Yang
2015-09-22  7:48   ` [PATCH v5 09/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang
2015-09-22  7:48   ` [PATCH v5 10/17] phy: Add driver for rockchip Display Port PHY Yakir Yang
2015-09-22  7:51   ` [PATCH v5 11/17] Documentation: phy: add document for rockchip dp phy Yakir Yang
2015-09-22  7:55   ` [PATCH v5 12/17] drm: rockchip: vop: add bpc and color mode setting Yakir Yang
2015-09-22  7:57   ` [PATCH v5 13/17] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang
2015-09-22  8:00   ` [PATCH v5 14/17] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang
2015-09-22  8:02   ` [PATCH v5 15/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang
2015-09-22  8:05   ` [PATCH v5 16/17] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang
2015-09-22  8:07   ` [PATCH v5 17/17] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang
     [not found]   ` <1442906963-2883-1-git-send-email-ykk@rock-chips.com>
2015-09-30  5:17     ` [PATCH v5 02/17] drm: bridge: analogix/dp: split exynos dp driver to bridge directory Krzysztof Kozlowski
2015-09-30  6:48       ` Yakir Yang
2015-10-07  6:25   ` [PATCH v5 0/17] Add Analogix Core Display Port Driver Yakir Yang
2015-10-07  8:46     ` Javier Martinez Canillas
2015-10-07  9:02       ` Yakir Yang
2015-10-07  9:26         ` Javier Martinez Canillas
     [not found]           ` <5614FC6B.4080702@rock-chips.com>
2015-10-07 11:25             ` Javier Martinez Canillas
2015-10-08  0:40               ` Yakir Yang
2015-10-10 14:31                 ` Yakir Yang
2015-10-13  9:21                   ` Javier Martinez Canillas
2015-10-13 13:50                     ` Yakir Yang
2015-10-14  8:18                       ` Javier Martinez Canillas
2015-10-10 15:35 ` [PATCH v6 " Yakir Yang
2015-10-10 15:38   ` [PATCH v6 01/17] drm: exynos: dp: convert to drm bridge mode Yakir Yang
2015-10-10 15:41   ` [PATCH v6 03/17] drm: bridge: analogix/dp: fix some obvious code style Yakir Yang
2015-10-10 15:43   ` [PATCH v6 04/17] drm: bridge: analogix/dp: remove duplicate configuration of link rate and link count Yakir Yang
2015-10-10 15:46   ` [PATCH v6 05/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range Yakir Yang
2015-10-12  0:37     ` Yakir Yang
2015-10-12  0:49       ` Krzysztof Kozlowski
2015-10-12  2:43         ` Yakir Yang
2015-10-12  3:51           ` Krzysztof Kozlowski
2015-10-12  4:09             ` Yakir Yang
2015-10-12  4:16               ` Krzysztof Kozlowski
2015-10-10 15:49   ` [PATCH v6 06/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang
2015-10-10 15:49   ` [PATCH v6 07/17] ARM: dts: exynos/dp: remove some properties that deprecated by analogix_dp driver Yakir Yang
2015-10-10 15:51   ` [PATCH v6 08/17] drm: rockchip: dp: add rockchip platform dp driver Yakir Yang
2015-10-10 15:53   ` [PATCH v6 09/17] Documentation: drm/bridge: add document for analogix_dp Yakir Yang
2015-10-10 15:55   ` [PATCH v6 10/17] phy: Add driver for rockchip Display Port PHY Yakir Yang
2015-10-12 15:02     ` Kishon Vijay Abraham I
2015-10-12 16:18       ` Heiko Stübner
2015-10-13  1:20       ` Yakir Yang
2015-10-10 15:58   ` [PATCH v6 11/17] Documentation: phy: add document for rockchip dp phy Yakir Yang
2015-10-12 22:28     ` Kishon Vijay Abraham I
2015-10-13  1:21       ` Yakir Yang
2015-10-10 16:00   ` [PATCH v6 12/17] drm: rockchip: vop: add bpc and color mode setting Yakir Yang
2015-10-10 16:05   ` [PATCH v6 13/17] drm: bridge: analogix/dp: add some rk3288 special registers setting Yakir Yang
2015-10-10 16:05   ` [PATCH v6 14/17] drm: bridge: analogix/dp: add max link rate and lane count limit for RK3288 Yakir Yang
2015-10-10 16:05   ` [PATCH v6 15/17] drm: bridge: analogix/dp: try force hpd after plug in lookup failed Yakir Yang
2015-10-10 16:05   ` [PATCH v6 16/17] drm: bridge: analogix/dp: move hpd detect to connector detect function Yakir Yang
2015-10-10 16:06   ` [PATCH v6 17/17] drm: bridge: analogix/dp: add edid modes parse in get_modes method Yakir Yang
2015-10-12  4:29   ` [PATCH v7 05/17] drm: bridge: analogix/dp: dynamic parse sync_pol & interlace & dynamic_range Yakir Yang
2015-10-12  6:54     ` Krzysztof Kozlowski
2015-10-12  7:20       ` Yakir Yang
2015-10-19 10:40   ` [PATCH v6 0/17] Add Analogix Core Display Port Driver Javier Martinez Canillas
2015-10-20  2:10     ` Yakir Yang
2015-10-20  9:48       ` Javier Martinez Canillas
2015-10-20 11:40         ` Yakir Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560B9CAA.9080109@samsung.com \
    --to=k.kozlowski@samsung$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox