On 21.03.2017 21:37, Sean Paul wrote:
> On Thu, Mar 16, 2017 at 03:14:28PM +0100, Andrzej Hajda wrote:
>> On 10.03.2017 05:32, Sean Paul wrote:
>>> From: zain wang <wzz@xxxxxxxxxxxxxx>
>>>
>>> We would meet a short black screen when exit PSR with the full link
>>> training, In this case, we should use fast link train instead of full
>>> link training.
>>>
>>> Signed-off-by: zain wang <wzz@xxxxxxxxxxxxxx>
>>> Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142 ++++++++++++++++-----
>>> drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +
>>> 2 files changed, 114 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> index 64d94a34874d..5bc151b0995b 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> @@ -10,17 +10,18 @@
>>> * option) any later version.
>>> */
>>>
>>> -#include <linux/module.h>
>>> -#include <linux/platform_device.h>
>>> -#include <linux/err.h>
>>> #include <linux/clk.h>
>>> -#include <linux/io.h>
>>> +#include <linux/component.h>
>>> +#include <linux/err.h>
>>> +#include <linux/gpio.h>
>>> #include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/module.h>
>>> #include <linux/of.h>
>>> #include <linux/of_gpio.h>
>>> -#include <linux/gpio.h>
>>> -#include <linux/component.h>
>>> #include <linux/phy/phy.h>
>>> +#include <linux/platform_device.h>
>>>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_atomic_helper.h>
>>> @@ -35,6 +36,8 @@
>>>
>>> #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
>>>
>>> +static const bool verify_fast_training;
>>> +
>> Quite odd debug sentinel, I am not sure if it is good practice to use
>> such construct.
> IIRC, they originally had a #define and then used #ifdef below to gate the
> verification. My concern with that was the code would rot if it wasn't compiled.
It just looks unusual, I am not strongly against current version.
>
> <snip>
>
>>> @@ -853,10 +933,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
>>> DRM_ERROR("failed to disable the panel\n");
>>> }
>>>
>>> - ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
>>> - dp->video_info.max_link_rate);
>>> + ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
>>> + DP_TIMEOUT_TRAINING_US * 5);
>>> if (ret) {
>>> - dev_err(dp->dev, "unable to do link train\n");
>>> + dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
>> And here we have ", ret=%d\n". Maybe it would be good to make it
>> consistent across whole driver.
> Meh. I don't think it's worth bikeshedding over commas in error messages. I'm
> just happy there are error checks/messages.
But if there will be next iteration it is not big deal to fix it then :)
Regards
Andrzej
>
>
>> Another issue with consistency in DRM_DEV_ERROR vs dev_err.
> Yeah, that's annoying.
>
> Sean
>
>> Regards
>> Andrzej
>>
>>> return;
>>> }
>>>
>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> index e135a42cb19e..920607d7eb3e 100644
>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>>> @@ -20,6 +20,8 @@
>>> #define MAX_CR_LOOP 5
>>> #define MAX_EQ_LOOP 5
>>>
>>> +/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */
>>> +#define DP_TIMEOUT_TRAINING_US 22000
>>> #define DP_TIMEOUT_PSR_LOOP_MS 300
>>>
>>> /* DP_MAX_LANE_COUNT */
>>> @@ -171,6 +173,7 @@ struct analogix_dp_device {
>>> int hpd_gpio;
>>> bool force_hpd;
>>> bool psr_enable;
>>> + bool fast_train_support;
>>>
>>> struct mutex panel_lock;
>>> bool panel_is_modeset;
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
