All of lore.kernel.org
 help / color / mirror / Atom feed
From: Torsten Duwe <duwe@lst.de>
To: Icenowy Zheng <icenowy@aosc.io>, Thomas Zimmermann <tzimmermann@suse.de>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Jonas Karlman <jonas@kwiboo.se>,
	Neil Armstrong <narmstrong@baylibre.com>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	Vasily Khoruzhick <anarsoul@gmail.com>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Maxime Ripard <maxime@cerno.tech>
Subject: Re: [PATCH] drm/bridge: analogix-anx6345: fix set of link bandwidth
Date: Mon, 24 Feb 2020 11:44:52 +0100	[thread overview]
Message-ID: <20200224104452.GA31184@lst.de> (raw)
In-Reply-To: <1E7BDB0F-639B-42BB-A4B4-A4C8CF94EBE0@aosc.io>

On Sat, Feb 22, 2020 at 10:43:02AM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2020年2月22日 GMT+08:00 上午1:13:28, Torsten Duwe <duwe@lst.de> 写到:
> >On Sat, Feb 22, 2020 at 12:51:27AM +0800, Icenowy Zheng wrote:
> >> Current code tries to store the link rate (in bps, which is a big
> >> number) in a u8, which surely overflow. Then it's converted back to
> >> bandwidth code (which is thus 0) and written to the chip.
> >> 
> >> The code sometimes works because the chip will automatically fallback
> >to
> >> the lowest possible DP link rate (1.62Gbps) when get the invalid
> >value.
> >> However, on the eDP panel of Olimex TERES-I, which wants 2.7Gbps
> >link,
> >> it failed.
> >> 
> >> As we had already read the link bandwidth as bandwidth code in
> >earlier
> >> code (to check whether it is supported), use it when setting
> >bandwidth,
> >> instead of converting it to link rate and then converting back.
> >> 
> >> Fixes: e1cff82c1097 ("drm/bridge: fix anx6345 compilation for v5.5")
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> index 56f55c53abfd..2dfa2fd2a23b 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> @@ -210,8 +210,7 @@ static int anx6345_dp_link_training(struct
> >anx6345 *anx6345)
> >>  	if (err)
> >>  		return err;
> >>  
> >> -	dpcd[0] = drm_dp_max_link_rate(anx6345->dpcd);
> >> -	dpcd[0] = drm_dp_link_rate_to_bw_code(dpcd[0]);
> >> +	dpcd[0] = dp_bw;
> >
> >Why do you make this assignment and not use dp_bw directly in the call?
> 
> Because the dpcd array is then written as a continous array
> back to DPCD.

But the current code never changes this value?
Anyway, as this might change in the future, I support your version;
I want to see this fixed.

Reviewed-by: Torsten Duwe <duwe@suse.de>
Fixes: e1cff82c1097 ("drm/bridge: fix anx6345 compilation for v5.5")
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Torsten Duwe <duwe@lst.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Icenowy Zheng <icenowy@aosc.io>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>

> >
> >>  	err = regmap_write(anx6345->map[I2C_IDX_DPTX],
> >>  			   SP_DP_MAIN_LINK_BW_SET_REG, dpcd[0]);
> >                                                       ^^^^^^
> >>  	if (err)
> >> -- 
> >> 2.24.1
> >
> >BTW, my version is only a bit more verbose:
> >
> >https://patchwork.freedesktop.org/patch/354344/
> >
> >	Torsten
> 
> -- 
> 使用 K-9 Mail 发送自我的Android设备。
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Torsten Duwe <duwe@lst.de>
To: Icenowy Zheng <icenowy@aosc.io>, Thomas Zimmermann <tzimmermann@suse.de>
Cc: Andrzej Hajda <a.hajda@samsung.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Maxime Ripard <maxime@cerno.tech>,
	Vasily Khoruzhick <anarsoul@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/bridge: analogix-anx6345: fix set of link bandwidth
Date: Mon, 24 Feb 2020 11:44:52 +0100	[thread overview]
Message-ID: <20200224104452.GA31184@lst.de> (raw)
In-Reply-To: <1E7BDB0F-639B-42BB-A4B4-A4C8CF94EBE0@aosc.io>

On Sat, Feb 22, 2020 at 10:43:02AM +0800, Icenowy Zheng wrote:
> 
> 
> 于 2020年2月22日 GMT+08:00 上午1:13:28, Torsten Duwe <duwe@lst.de> 写到:
> >On Sat, Feb 22, 2020 at 12:51:27AM +0800, Icenowy Zheng wrote:
> >> Current code tries to store the link rate (in bps, which is a big
> >> number) in a u8, which surely overflow. Then it's converted back to
> >> bandwidth code (which is thus 0) and written to the chip.
> >> 
> >> The code sometimes works because the chip will automatically fallback
> >to
> >> the lowest possible DP link rate (1.62Gbps) when get the invalid
> >value.
> >> However, on the eDP panel of Olimex TERES-I, which wants 2.7Gbps
> >link,
> >> it failed.
> >> 
> >> As we had already read the link bandwidth as bandwidth code in
> >earlier
> >> code (to check whether it is supported), use it when setting
> >bandwidth,
> >> instead of converting it to link rate and then converting back.
> >> 
> >> Fixes: e1cff82c1097 ("drm/bridge: fix anx6345 compilation for v5.5")
> >> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> >> ---
> >>  drivers/gpu/drm/bridge/analogix/analogix-anx6345.c | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> index 56f55c53abfd..2dfa2fd2a23b 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx6345.c
> >> @@ -210,8 +210,7 @@ static int anx6345_dp_link_training(struct
> >anx6345 *anx6345)
> >>  	if (err)
> >>  		return err;
> >>  
> >> -	dpcd[0] = drm_dp_max_link_rate(anx6345->dpcd);
> >> -	dpcd[0] = drm_dp_link_rate_to_bw_code(dpcd[0]);
> >> +	dpcd[0] = dp_bw;
> >
> >Why do you make this assignment and not use dp_bw directly in the call?
> 
> Because the dpcd array is then written as a continous array
> back to DPCD.

But the current code never changes this value?
Anyway, as this might change in the future, I support your version;
I want to see this fixed.

Reviewed-by: Torsten Duwe <duwe@suse.de>
Fixes: e1cff82c1097 ("drm/bridge: fix anx6345 compilation for v5.5")
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Torsten Duwe <duwe@lst.de>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Icenowy Zheng <icenowy@aosc.io>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>

> >
> >>  	err = regmap_write(anx6345->map[I2C_IDX_DPTX],
> >>  			   SP_DP_MAIN_LINK_BW_SET_REG, dpcd[0]);
> >                                                       ^^^^^^
> >>  	if (err)
> >> -- 
> >> 2.24.1
> >
> >BTW, my version is only a bit more verbose:
> >
> >https://patchwork.freedesktop.org/patch/354344/
> >
> >	Torsten
> 
> -- 
> 使用 K-9 Mail 发送自我的Android设备。

  reply	other threads:[~2020-02-24 10:45 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-21 16:51 [PATCH] drm/bridge: analogix-anx6345: fix set of link bandwidth Icenowy Zheng
2020-02-21 16:51 ` Icenowy Zheng
2020-02-21 17:13 ` Torsten Duwe
2020-02-21 17:13   ` Torsten Duwe
2020-02-22  2:43   ` Icenowy Zheng
2020-02-22  2:43     ` Icenowy Zheng
2020-02-24 10:44     ` Torsten Duwe [this message]
2020-02-24 10:44       ` Torsten Duwe
2020-02-26 10:58     ` Thomas Zimmermann
2020-02-26 10:58       ` Thomas Zimmermann
2020-02-26 11:02       ` Icenowy Zheng
2020-02-26 11:02         ` Icenowy Zheng
2020-02-26 11:59         ` Thomas Zimmermann
2020-02-26 11:59           ` Thomas Zimmermann
2020-02-27 11:34         ` Thomas Zimmermann
2020-02-27 11:34           ` Thomas Zimmermann

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=20200224104452.GA31184@lst.de \
    --to=duwe@lst.de \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=a.hajda@samsung.com \
    --cc=anarsoul@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=icenowy@aosc.io \
    --cc=jernej.skrabec@siol.net \
    --cc=jonas@kwiboo.se \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime@cerno.tech \
    --cc=narmstrong@baylibre.com \
    --cc=sam@ravnborg.org \
    --cc=sfr@canb.auug.org.au \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.