| Message ID | 20240322104732.2327060-1-sean@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/stm: dsi: relax mode_valid clock tolerance | expand |
On Fri, Mar 22, 2024 at 11:47:31AM +0100, Sean Nyekjaer wrote: > When using the DSI interface via DSI2LVDS bridge, it seems a bit harsh > to reguire the requested and the actual px clock to be within > 50Hz. A typical LVDS display requires the px clock to be within +-10%. > > In case for HDMI .5% tolerance is required. > > Fixes: e01356d18273 ("drm/stm: dsi: provide the implementation of mode_valid()") > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- Any feedback on this? > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > index d5f8c923d7bc..97936b0ef702 100644 > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > @@ -322,8 +322,6 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, > return 0; > } > > -#define CLK_TOLERANCE_HZ 50 > - > static enum drm_mode_status > dw_mipi_dsi_stm_mode_valid(void *priv_data, > const struct drm_display_mode *mode, > @@ -375,9 +373,10 @@ dw_mipi_dsi_stm_mode_valid(void *priv_data, > /* > * Filter modes according to the clock value, particularly useful for > * hdmi modes that require precise pixel clocks. > + * Check that px_clock is within .5% tolerance. > */ > - if (px_clock_hz < target_px_clock_hz - CLK_TOLERANCE_HZ || > - px_clock_hz > target_px_clock_hz + CLK_TOLERANCE_HZ) > + if (px_clock_hz < mult_frac(target_px_clock_hz, 995, 1000) || > + px_clock_hz > mult_frac(target_px_clock_hz, 1005, 1000)) > return MODE_CLOCK_RANGE; > > /* sync packets are codes as DSI short packets (4 bytes) */ > -- > 2.44.0 >
On Mon, Apr 22, 2024 at 4:06 PM Sean Nyekjaer <sean@geanix.com> wrote: > > On Fri, Mar 22, 2024 at 11:47:31AM +0100, Sean Nyekjaer wrote: > > When using the DSI interface via DSI2LVDS bridge, it seems a bit harsh > > to reguire the requested and the actual px clock to be within > > 50Hz. A typical LVDS display requires the px clock to be within +-10%. > > > > In case for HDMI .5% tolerance is required. > > > > Fixes: e01356d18273 ("drm/stm: dsi: provide the implementation of mode_valid()") > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > > --- > Any feedback on this? > > > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > > index d5f8c923d7bc..97936b0ef702 100644 > > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > > @@ -322,8 +322,6 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, > > return 0; > > } > > > > -#define CLK_TOLERANCE_HZ 50 > > - > > static enum drm_mode_status > > dw_mipi_dsi_stm_mode_valid(void *priv_data, > > const struct drm_display_mode *mode, > > @@ -375,9 +373,10 @@ dw_mipi_dsi_stm_mode_valid(void *priv_data, > > /* > > * Filter modes according to the clock value, particularly useful for > > * hdmi modes that require precise pixel clocks. > > + * Check that px_clock is within .5% tolerance. > > */ > > - if (px_clock_hz < target_px_clock_hz - CLK_TOLERANCE_HZ || > > - px_clock_hz > target_px_clock_hz + CLK_TOLERANCE_HZ) > > + if (px_clock_hz < mult_frac(target_px_clock_hz, 995, 1000) || > > + px_clock_hz > mult_frac(target_px_clock_hz, 1005, 1000)) > > return MODE_CLOCK_RANGE; > > > > /* sync packets are codes as DSI short packets (4 bytes) */ > > -- > > 2.44.0 > > > Reviewed-by: Robert Foss <rfoss@kernel.org>
Hi, Sorry, my previous review didn't go through. On Fri, Mar 22, 2024 at 11:47:31AM +0100, Sean Nyekjaer wrote: > When using the DSI interface via DSI2LVDS bridge, it seems a bit harsh > to reguire the requested and the actual px clock to be within > 50Hz. A typical LVDS display requires the px clock to be within +-10%. > > In case for HDMI .5% tolerance is required. > > Fixes: e01356d18273 ("drm/stm: dsi: provide the implementation of mode_valid()") > Signed-off-by: Sean Nyekjaer <sean@geanix.com> > --- > drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > index d5f8c923d7bc..97936b0ef702 100644 > --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c > @@ -322,8 +322,6 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, > return 0; > } > > -#define CLK_TOLERANCE_HZ 50 > - > static enum drm_mode_status > dw_mipi_dsi_stm_mode_valid(void *priv_data, > const struct drm_display_mode *mode, > @@ -375,9 +373,10 @@ dw_mipi_dsi_stm_mode_valid(void *priv_data, > /* > * Filter modes according to the clock value, particularly useful for > * hdmi modes that require precise pixel clocks. > + * Check that px_clock is within .5% tolerance. > */ > - if (px_clock_hz < target_px_clock_hz - CLK_TOLERANCE_HZ || > - px_clock_hz > target_px_clock_hz + CLK_TOLERANCE_HZ) > + if (px_clock_hz < mult_frac(target_px_clock_hz, 995, 1000) || > + px_clock_hz > mult_frac(target_px_clock_hz, 1005, 1000)) > return MODE_CLOCK_RANGE; I wonder if it's not something that should be made into a helper. We have a couple of drivers doing it already, so it might be worth creating a function that checks for a given struct clk pointer and pixel clock if it's within parameters. Maxime
diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c index d5f8c923d7bc..97936b0ef702 100644 --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c @@ -322,8 +322,6 @@ dw_mipi_dsi_phy_get_timing(void *priv_data, unsigned int lane_mbps, return 0; } -#define CLK_TOLERANCE_HZ 50 - static enum drm_mode_status dw_mipi_dsi_stm_mode_valid(void *priv_data, const struct drm_display_mode *mode, @@ -375,9 +373,10 @@ dw_mipi_dsi_stm_mode_valid(void *priv_data, /* * Filter modes according to the clock value, particularly useful for * hdmi modes that require precise pixel clocks. + * Check that px_clock is within .5% tolerance. */ - if (px_clock_hz < target_px_clock_hz - CLK_TOLERANCE_HZ || - px_clock_hz > target_px_clock_hz + CLK_TOLERANCE_HZ) + if (px_clock_hz < mult_frac(target_px_clock_hz, 995, 1000) || + px_clock_hz > mult_frac(target_px_clock_hz, 1005, 1000)) return MODE_CLOCK_RANGE; /* sync packets are codes as DSI short packets (4 bytes) */
When using the DSI interface via DSI2LVDS bridge, it seems a bit harsh to reguire the requested and the actual px clock to be within 50Hz. A typical LVDS display requires the px clock to be within +-10%. In case for HDMI .5% tolerance is required. Fixes: e01356d18273 ("drm/stm: dsi: provide the implementation of mode_valid()") Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)