* [PATCH] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535
@ 2023-03-11 23:10 Adam Ford
2023-03-12 1:10 ` Dmitry Baryshkov
0 siblings, 1 reply; 3+ messages in thread
From: Adam Ford @ 2023-03-11 23:10 UTC (permalink / raw)
To: dri-devel
Cc: aford, dmitry.baryshkov, quic_abhinavk, Adam Ford, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, linux-kernel
When dynamically switching lanes was removed, the intent of the code
was to check to make sure that higher speed items used 4 lanes, but
it had the unintended consequence of removing the slower speeds for
4-lane users.
This attempts to remedy this by doing a check to see that the
max frequency doesn't exceed the chip limit, and a second
check to make sure that the max bit-rate doesn't exceed the
number of lanes * max bit rate / lane.
Fixes: 9a0cdcd6649b ("drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge")
Signed-off-by: Adam Ford <aford173@gmail.com>
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index fdfeadcefe80..10a112d36945 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -103,13 +103,9 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
const struct drm_display_mode *mode)
{
- int lanes;
+ unsigned long max_lane_freq;
struct mipi_dsi_device *dsi = adv->dsi;
-
- if (mode->clock > 80000)
- lanes = 4;
- else
- lanes = 3;
+ u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
/*
* TODO: add support for dynamic switching of lanes
@@ -117,8 +113,16 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
* out the modes which shall need different number of lanes
* than what was configured in the device tree.
*/
- if (lanes != dsi->lanes)
- return MODE_BAD;
+
+ /* Check max clock for either 7533 or 7535 */
+ if (mode->clock > (adv->type == ADV7533 ? 80000 : 148500))
+ return MODE_CLOCK_HIGH;
+
+ /* Check max clock for each lane */
+ max_lane_freq = (adv->type == ADV7533 ? 800000 : 891000);
+
+ if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
+ return MODE_CLOCK_HIGH;
return MODE_OK;
}
--
2.37.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535
2023-03-11 23:10 [PATCH] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535 Adam Ford
@ 2023-03-12 1:10 ` Dmitry Baryshkov
2023-03-13 13:56 ` Robert Foss
0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Baryshkov @ 2023-03-12 1:10 UTC (permalink / raw)
To: Adam Ford, dri-devel
Cc: aford, quic_abhinavk, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, David Airlie,
Daniel Vetter, linux-kernel
On 12/03/2023 01:10, Adam Ford wrote:
> When dynamically switching lanes was removed, the intent of the code
> was to check to make sure that higher speed items used 4 lanes, but
> it had the unintended consequence of removing the slower speeds for
> 4-lane users.
>
> This attempts to remedy this by doing a check to see that the
> max frequency doesn't exceed the chip limit, and a second
> check to make sure that the max bit-rate doesn't exceed the
> number of lanes * max bit rate / lane.
>
> Fixes: 9a0cdcd6649b ("drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge")
>
Please remove the empty line here. There should be no empty lines
between the tags
> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> index fdfeadcefe80..10a112d36945 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> @@ -103,13 +103,9 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
> const struct drm_display_mode *mode)
> {
> - int lanes;
> + unsigned long max_lane_freq;
> struct mipi_dsi_device *dsi = adv->dsi;
> -
> - if (mode->clock > 80000)
> - lanes = 4;
> - else
> - lanes = 3;
> + u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>
> /*
> * TODO: add support for dynamic switching of lanes
Drop the comment please. It makes little sense with your new implementation.
> @@ -117,8 +113,16 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
> * out the modes which shall need different number of lanes
> * than what was configured in the device tree.
> */
> - if (lanes != dsi->lanes)
> - return MODE_BAD;
> +
> + /* Check max clock for either 7533 or 7535 */
> + if (mode->clock > (adv->type == ADV7533 ? 80000 : 148500))
> + return MODE_CLOCK_HIGH;
> +
> + /* Check max clock for each lane */
> + max_lane_freq = (adv->type == ADV7533 ? 800000 : 891000);
> +
> + if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
> + return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> }
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535
2023-03-12 1:10 ` Dmitry Baryshkov
@ 2023-03-13 13:56 ` Robert Foss
0 siblings, 0 replies; 3+ messages in thread
From: Robert Foss @ 2023-03-13 13:56 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Adam Ford, dri-devel, aford, quic_abhinavk, Andrzej Hajda,
Neil Armstrong, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
David Airlie, Daniel Vetter, linux-kernel
On Sun, Mar 12, 2023 at 2:10 AM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/03/2023 01:10, Adam Ford wrote:
> > When dynamically switching lanes was removed, the intent of the code
> > was to check to make sure that higher speed items used 4 lanes, but
> > it had the unintended consequence of removing the slower speeds for
> > 4-lane users.
> >
> > This attempts to remedy this by doing a check to see that the
> > max frequency doesn't exceed the chip limit, and a second
> > check to make sure that the max bit-rate doesn't exceed the
> > number of lanes * max bit rate / lane.
> >
> > Fixes: 9a0cdcd6649b ("drm/bridge: adv7533: remove dynamic lane switching from adv7533 bridge")
> >
>
> Please remove the empty line here. There should be no empty lines
> between the tags
>
>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > index fdfeadcefe80..10a112d36945 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > @@ -103,13 +103,9 @@ void adv7533_dsi_power_off(struct adv7511 *adv)
> > enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
> > const struct drm_display_mode *mode)
> > {
> > - int lanes;
> > + unsigned long max_lane_freq;
> > struct mipi_dsi_device *dsi = adv->dsi;
> > -
> > - if (mode->clock > 80000)
> > - lanes = 4;
> > - else
> > - lanes = 3;
> > + u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> >
> > /*
> > * TODO: add support for dynamic switching of lanes
>
> Drop the comment please. It makes little sense with your new implementation.
>
> > @@ -117,8 +113,16 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
> > * out the modes which shall need different number of lanes
> > * than what was configured in the device tree.
> > */
> > - if (lanes != dsi->lanes)
> > - return MODE_BAD;
> > +
> > + /* Check max clock for either 7533 or 7535 */
> > + if (mode->clock > (adv->type == ADV7533 ? 80000 : 148500))
> > + return MODE_CLOCK_HIGH;
> > +
> > + /* Check max clock for each lane */
> > + max_lane_freq = (adv->type == ADV7533 ? 800000 : 891000);
> > +
> > + if (mode->clock * bpp > max_lane_freq * adv->num_dsi_lanes)
> > + return MODE_CLOCK_HIGH;
> >
> > return MODE_OK;
> > }
>
> --
> With best wishes
> Dmitry
>
With Dmitrys feedback, feel free to add my Reviewed-by.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-13 13:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-11 23:10 [PATCH] drm/bridge: adv7533: Fix adv7533_mode_valid for adv7533 and adv7535 Adam Ford
2023-03-12 1:10 ` Dmitry Baryshkov
2023-03-13 13:56 ` Robert Foss
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox