| Message ID | 20230423121232.1345909-5-aford173@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: bridge: samsung-dsim: Support variable clocking | expand |
On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: > > The DPHY timings are currently hard coded. Since the input > clock can be variable, the phy timings need to be variable > too. Add an additional variable to the driver data to enable > this feature to prevent breaking boards that don't support it. > > The phy_mipi_dphy_get_default_config function configures the > DPHY timings in pico-seconds, and a small macro converts those > timings into clock cycles based on the pixel clock rate. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- > include/drm/bridge/samsung-dsim.h | 1 + > 2 files changed, 70 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > index 5b6e7825b92f..f165483d5044 100644 > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > @@ -18,9 +18,7 @@ > #include <linux/media-bus-format.h> > #include <linux/of_device.h> > #include <linux/phy/phy.h> > - > #include <video/mipi_display.h> > - > #include <drm/bridge/samsung-dsim.h> > #include <drm/drm_panel.h> > #include <drm/drm_print.h> > @@ -218,6 +216,8 @@ > > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > > +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) > + > static const char *const clk_names[5] = { > "bus_clk", > "sclk_mipi", > @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { > .m_min = 64, > .m_max = 1023, > .vco_min = 1050, > + .dynamic_dphy = 1, > }; > > static const struct samsung_dsim_driver_data * > @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > const unsigned int *reg_values = driver_data->reg_values; > u32 reg; > + struct drm_display_mode *m = &dsi->mode; > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > + struct phy_configure_opts_mipi_dphy cfg; > + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > + int hs_exit, hs_prepare, hs_zero, hs_trail; > + unsigned long long clock_in_hz = m->clock * 1000; > > if (driver_data->has_freqband) > return; > > + /* The dynamic_phy has the ability to adjust PHY Timing settings */ > + if (driver_data->dynamic_dphy) { > + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: ERROR: modpost: "phy_mipi_dphy_get_default_config" [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 make[4]: *** [Makefile:1978: modpost] Error 2 make[3]: *** [Makefile:357: __build_one_by_one] Error 2 I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. > + > + /* > + * TODO: > + * The tech reference manual for i.MX8M Mini/Nano/Plus > + * doesn't state what the definition of the PHYTIMING > + * bits are beyond their address and bit position. > + * After reviewing NXP's downstream code, it appears > + * that the various PHYTIMING registers take the number > + * of cycles and use various dividers on them. This > + * calculation does not result in an exact match to the > + * downstream code, but it is very close, and it appears > + * to sync at a variety of resolutions. If someone > + * can get a more accurate mathematical equation needed > + * for these registers, this should be updated. > + */ > + > + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz); > + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz); > + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz); > + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz); > + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz); > + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz); > + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz); > + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz); > + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz); > + } > + > /* B D-PHY: D-PHY Master & Slave Analog Block control */ > reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] | > reg_values[PHYCTRL_SLEW_UP]; > + > samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg); > > /* > @@ -712,7 +750,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > * T HS-EXIT: Time that the transmitter drives LP-11 following a HS > * burst > */ > - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; > + if (driver_data->dynamic_dphy) > + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit); > + else > + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; > + > samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg); > > /* > @@ -728,10 +770,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after > * the last payload clock bit of a HS transmission burst > */ > - reg = reg_values[PHYTIMING_CLK_PREPARE] | > - reg_values[PHYTIMING_CLK_ZERO] | > - reg_values[PHYTIMING_CLK_POST] | > - reg_values[PHYTIMING_CLK_TRAIL]; > + if (driver_data->dynamic_dphy) { > + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) | > + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) | > + DSIM_PHYTIMING1_CLK_POST(clk_post) | > + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail); > + } else { > + reg = reg_values[PHYTIMING_CLK_PREPARE] | > + reg_values[PHYTIMING_CLK_ZERO] | > + reg_values[PHYTIMING_CLK_POST] | > + reg_values[PHYTIMING_CLK_TRAIL]; > + } > > samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg); > > @@ -744,8 +793,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > * T HS-TRAIL: Time that the transmitter drives the flipped differential > * state after last payload data bit of a HS transmission burst > */ > - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] | > - reg_values[PHYTIMING_HS_TRAIL]; > + > + if (driver_data->dynamic_dphy) { > + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) | > + DSIM_PHYTIMING2_HS_ZERO(hs_zero) | > + DSIM_PHYTIMING2_HS_TRAIL(hs_trail); > + } else { > + reg = reg_values[PHYTIMING_HS_PREPARE] | > + reg_values[PHYTIMING_HS_ZERO] | > + reg_values[PHYTIMING_HS_TRAIL]; > + } > + > samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg); > } > > @@ -1337,7 +1395,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi) > samsung_dsim_enable_clock(dsi); > if (driver_data->wait_for_reset) > samsung_dsim_wait_for_reset(dsi); > - samsung_dsim_set_phy_ctrl(dsi); > + if (!driver_data->has_freqband) > + samsung_dsim_set_phy_ctrl(dsi); > samsung_dsim_init_link(dsi); > > dsi->state |= DSIM_STATE_INITIALIZED; > diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h > index a088d84579bc..25475d78adb3 100644 > --- a/include/drm/bridge/samsung-dsim.h > +++ b/include/drm/bridge/samsung-dsim.h > @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data { > u16 m_min; > u16 m_max; > u64 vco_min; > + bool dynamic_dphy; > }; > > struct samsung_dsim_host_ops { > -- > 2.39.2 >
On Mon, Apr 24, 2023 at 1:12 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: > > > > The DPHY timings are currently hard coded. Since the input > > clock can be variable, the phy timings need to be variable > > too. Add an additional variable to the driver data to enable > > this feature to prevent breaking boards that don't support it. > > > > The phy_mipi_dphy_get_default_config function configures the > > DPHY timings in pico-seconds, and a small macro converts those > > timings into clock cycles based on the pixel clock rate. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > --- > > drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- > > include/drm/bridge/samsung-dsim.h | 1 + > > 2 files changed, 70 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > > index 5b6e7825b92f..f165483d5044 100644 > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > @@ -18,9 +18,7 @@ > > #include <linux/media-bus-format.h> > > #include <linux/of_device.h> > > #include <linux/phy/phy.h> > > - > > #include <video/mipi_display.h> > > - > > #include <drm/bridge/samsung-dsim.h> > > #include <drm/drm_panel.h> > > #include <drm/drm_print.h> > > @@ -218,6 +216,8 @@ > > > > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > > > > +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) > > + > > static const char *const clk_names[5] = { > > "bus_clk", > > "sclk_mipi", > > @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { > > .m_min = 64, > > .m_max = 1023, > > .vco_min = 1050, > > + .dynamic_dphy = 1, > > }; > > > > static const struct samsung_dsim_driver_data * > > @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > > const unsigned int *reg_values = driver_data->reg_values; > > u32 reg; > > + struct drm_display_mode *m = &dsi->mode; > > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > > + struct phy_configure_opts_mipi_dphy cfg; > > + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > > + int hs_exit, hs_prepare, hs_zero, hs_trail; > > + unsigned long long clock_in_hz = m->clock * 1000; > > > > if (driver_data->has_freqband) > > return; > > > > + /* The dynamic_phy has the ability to adjust PHY Timing settings */ > > + if (driver_data->dynamic_dphy) { > > + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); > > This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, > otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: > > ERROR: modpost: "phy_mipi_dphy_get_default_config" > [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! > make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 > make[4]: *** [Makefile:1978: modpost] Error 2 > make[3]: *** [Makefile:357: __build_one_by_one] Error 2 > > I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. That's interesting, I didn't come across that. What did you use for a starting point when you applied the patches? I want to see if I can replicate it. adam > > > + > > + /* > > + * TODO: > > + * The tech reference manual for i.MX8M Mini/Nano/Plus > > + * doesn't state what the definition of the PHYTIMING > > + * bits are beyond their address and bit position. > > + * After reviewing NXP's downstream code, it appears > > + * that the various PHYTIMING registers take the number > > + * of cycles and use various dividers on them. This > > + * calculation does not result in an exact match to the > > + * downstream code, but it is very close, and it appears > > + * to sync at a variety of resolutions. If someone > > + * can get a more accurate mathematical equation needed > > + * for these registers, this should be updated. > > + */ > > + > > + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz); > > + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz); > > + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz); > > + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz); > > + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz); > > + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz); > > + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz); > > + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz); > > + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz); > > + } > > + > > /* B D-PHY: D-PHY Master & Slave Analog Block control */ > > reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] | > > reg_values[PHYCTRL_SLEW_UP]; > > + > > samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg); > > > > /* > > @@ -712,7 +750,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > * T HS-EXIT: Time that the transmitter drives LP-11 following a HS > > * burst > > */ > > - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; > > + if (driver_data->dynamic_dphy) > > + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit); > > + else > > + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; > > + > > samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg); > > > > /* > > @@ -728,10 +770,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after > > * the last payload clock bit of a HS transmission burst > > */ > > - reg = reg_values[PHYTIMING_CLK_PREPARE] | > > - reg_values[PHYTIMING_CLK_ZERO] | > > - reg_values[PHYTIMING_CLK_POST] | > > - reg_values[PHYTIMING_CLK_TRAIL]; > > + if (driver_data->dynamic_dphy) { > > + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) | > > + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) | > > + DSIM_PHYTIMING1_CLK_POST(clk_post) | > > + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail); > > + } else { > > + reg = reg_values[PHYTIMING_CLK_PREPARE] | > > + reg_values[PHYTIMING_CLK_ZERO] | > > + reg_values[PHYTIMING_CLK_POST] | > > + reg_values[PHYTIMING_CLK_TRAIL]; > > + } > > > > samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg); > > > > @@ -744,8 +793,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > * T HS-TRAIL: Time that the transmitter drives the flipped differential > > * state after last payload data bit of a HS transmission burst > > */ > > - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] | > > - reg_values[PHYTIMING_HS_TRAIL]; > > + > > + if (driver_data->dynamic_dphy) { > > + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) | > > + DSIM_PHYTIMING2_HS_ZERO(hs_zero) | > > + DSIM_PHYTIMING2_HS_TRAIL(hs_trail); > > + } else { > > + reg = reg_values[PHYTIMING_HS_PREPARE] | > > + reg_values[PHYTIMING_HS_ZERO] | > > + reg_values[PHYTIMING_HS_TRAIL]; > > + } > > + > > samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg); > > } > > > > @@ -1337,7 +1395,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi) > > samsung_dsim_enable_clock(dsi); > > if (driver_data->wait_for_reset) > > samsung_dsim_wait_for_reset(dsi); > > - samsung_dsim_set_phy_ctrl(dsi); > > + if (!driver_data->has_freqband) > > + samsung_dsim_set_phy_ctrl(dsi); > > samsung_dsim_init_link(dsi); > > > > dsi->state |= DSIM_STATE_INITIALIZED; > > diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h > > index a088d84579bc..25475d78adb3 100644 > > --- a/include/drm/bridge/samsung-dsim.h > > +++ b/include/drm/bridge/samsung-dsim.h > > @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data { > > u16 m_min; > > u16 m_max; > > u64 vco_min; > > + bool dynamic_dphy; > > }; > > > > struct samsung_dsim_host_ops { > > -- > > 2.39.2 > >
On Mon, Apr 24, 2023 at 5:31 PM Adam Ford <aford173@gmail.com> wrote: > > On Mon, Apr 24, 2023 at 1:12 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > > > On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > The DPHY timings are currently hard coded. Since the input > > > clock can be variable, the phy timings need to be variable > > > too. Add an additional variable to the driver data to enable > > > this feature to prevent breaking boards that don't support it. > > > > > > The phy_mipi_dphy_get_default_config function configures the > > > DPHY timings in pico-seconds, and a small macro converts those > > > timings into clock cycles based on the pixel clock rate. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > --- > > > drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- > > > include/drm/bridge/samsung-dsim.h | 1 + > > > 2 files changed, 70 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > > > index 5b6e7825b92f..f165483d5044 100644 > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > > @@ -18,9 +18,7 @@ > > > #include <linux/media-bus-format.h> > > > #include <linux/of_device.h> > > > #include <linux/phy/phy.h> > > > - > > > #include <video/mipi_display.h> > > > - > > > #include <drm/bridge/samsung-dsim.h> > > > #include <drm/drm_panel.h> > > > #include <drm/drm_print.h> > > > @@ -218,6 +216,8 @@ > > > > > > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > > > > > > +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) > > > + > > > static const char *const clk_names[5] = { > > > "bus_clk", > > > "sclk_mipi", > > > @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { > > > .m_min = 64, > > > .m_max = 1023, > > > .vco_min = 1050, > > > + .dynamic_dphy = 1, > > > }; > > > > > > static const struct samsung_dsim_driver_data * > > > @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > > const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > > > const unsigned int *reg_values = driver_data->reg_values; > > > u32 reg; > > > + struct drm_display_mode *m = &dsi->mode; > > > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > > > + struct phy_configure_opts_mipi_dphy cfg; > > > + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > > > + int hs_exit, hs_prepare, hs_zero, hs_trail; > > > + unsigned long long clock_in_hz = m->clock * 1000; > > > > > > if (driver_data->has_freqband) > > > return; > > > > > > + /* The dynamic_phy has the ability to adjust PHY Timing settings */ > > > + if (driver_data->dynamic_dphy) { > > > + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); > > > > This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, > > otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: > > > > ERROR: modpost: "phy_mipi_dphy_get_default_config" > > [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! > > make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 > > make[4]: *** [Makefile:1978: modpost] Error 2 > > make[3]: *** [Makefile:357: __build_one_by_one] Error 2 > > > > I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. > > That's interesting, I didn't come across that. > What did you use for a starting point when you applied the patches? > I want to see if I can replicate it. next-20230421. My config is pretty much tailored to the Hummingbird Pulse. Device drivers for other hardware or things that I can't enable are all disabled. For example I don't have PHY_MIXEL_MIPI_DPHY enabled. Maybe you have some other bridge or phy that selects it enabled? ChenYu
On 24.04.2023 11:44, Chen-Yu Tsai wrote: > On Mon, Apr 24, 2023 at 5:31 PM Adam Ford <aford173@gmail.com> wrote: >> On Mon, Apr 24, 2023 at 1:12 AM Chen-Yu Tsai <wenst@chromium.org> wrote: >>> On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: >>>> The DPHY timings are currently hard coded. Since the input >>>> clock can be variable, the phy timings need to be variable >>>> too. Add an additional variable to the driver data to enable >>>> this feature to prevent breaking boards that don't support it. >>>> >>>> The phy_mipi_dphy_get_default_config function configures the >>>> DPHY timings in pico-seconds, and a small macro converts those >>>> timings into clock cycles based on the pixel clock rate. >>>> >>>> Signed-off-by: Adam Ford <aford173@gmail.com> >>>> --- >>>> drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- >>>> include/drm/bridge/samsung-dsim.h | 1 + >>>> 2 files changed, 70 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c >>>> index 5b6e7825b92f..f165483d5044 100644 >>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c >>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c >>>> @@ -18,9 +18,7 @@ >>>> #include <linux/media-bus-format.h> >>>> #include <linux/of_device.h> >>>> #include <linux/phy/phy.h> >>>> - >>>> #include <video/mipi_display.h> >>>> - >>>> #include <drm/bridge/samsung-dsim.h> >>>> #include <drm/drm_panel.h> >>>> #include <drm/drm_print.h> >>>> @@ -218,6 +216,8 @@ >>>> >>>> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" >>>> >>>> +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) >>>> + >>>> static const char *const clk_names[5] = { >>>> "bus_clk", >>>> "sclk_mipi", >>>> @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { >>>> .m_min = 64, >>>> .m_max = 1023, >>>> .vco_min = 1050, >>>> + .dynamic_dphy = 1, >>>> }; >>>> >>>> static const struct samsung_dsim_driver_data * >>>> @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) >>>> const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; >>>> const unsigned int *reg_values = driver_data->reg_values; >>>> u32 reg; >>>> + struct drm_display_mode *m = &dsi->mode; >>>> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); >>>> + struct phy_configure_opts_mipi_dphy cfg; >>>> + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; >>>> + int hs_exit, hs_prepare, hs_zero, hs_trail; >>>> + unsigned long long clock_in_hz = m->clock * 1000; >>>> >>>> if (driver_data->has_freqband) >>>> return; >>>> >>>> + /* The dynamic_phy has the ability to adjust PHY Timing settings */ >>>> + if (driver_data->dynamic_dphy) { >>>> + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); >>> This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, >>> otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: >>> >>> ERROR: modpost: "phy_mipi_dphy_get_default_config" >>> [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! >>> make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 >>> make[4]: *** [Makefile:1978: modpost] Error 2 >>> make[3]: *** [Makefile:357: __build_one_by_one] Error 2 >>> >>> I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. >> That's interesting, I didn't come across that. >> What did you use for a starting point when you applied the patches? >> I want to see if I can replicate it. > next-20230421. My config is pretty much tailored to the Hummingbird Pulse. > Device drivers for other hardware or things that I can't enable are all > disabled. For example I don't have PHY_MIXEL_MIPI_DPHY enabled. > > Maybe you have some other bridge or phy that selects it enabled? I've observed similar issue while building exynos_defconfig for arm 32bit. Best regards
On Mon, Apr 24, 2023 at 4:47 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > On 24.04.2023 11:44, Chen-Yu Tsai wrote: > > On Mon, Apr 24, 2023 at 5:31 PM Adam Ford <aford173@gmail.com> wrote: > >> On Mon, Apr 24, 2023 at 1:12 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > >>> On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: > >>>> The DPHY timings are currently hard coded. Since the input > >>>> clock can be variable, the phy timings need to be variable > >>>> too. Add an additional variable to the driver data to enable > >>>> this feature to prevent breaking boards that don't support it. > >>>> > >>>> The phy_mipi_dphy_get_default_config function configures the > >>>> DPHY timings in pico-seconds, and a small macro converts those > >>>> timings into clock cycles based on the pixel clock rate. > >>>> > >>>> Signed-off-by: Adam Ford <aford173@gmail.com> > >>>> --- > >>>> drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- > >>>> include/drm/bridge/samsung-dsim.h | 1 + > >>>> 2 files changed, 70 insertions(+), 10 deletions(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > >>>> index 5b6e7825b92f..f165483d5044 100644 > >>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c > >>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > >>>> @@ -18,9 +18,7 @@ > >>>> #include <linux/media-bus-format.h> > >>>> #include <linux/of_device.h> > >>>> #include <linux/phy/phy.h> > >>>> - > >>>> #include <video/mipi_display.h> > >>>> - > >>>> #include <drm/bridge/samsung-dsim.h> > >>>> #include <drm/drm_panel.h> > >>>> #include <drm/drm_print.h> > >>>> @@ -218,6 +216,8 @@ > >>>> > >>>> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > >>>> > >>>> +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) > >>>> + > >>>> static const char *const clk_names[5] = { > >>>> "bus_clk", > >>>> "sclk_mipi", > >>>> @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { > >>>> .m_min = 64, > >>>> .m_max = 1023, > >>>> .vco_min = 1050, > >>>> + .dynamic_dphy = 1, > >>>> }; > >>>> > >>>> static const struct samsung_dsim_driver_data * > >>>> @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > >>>> const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > >>>> const unsigned int *reg_values = driver_data->reg_values; > >>>> u32 reg; > >>>> + struct drm_display_mode *m = &dsi->mode; > >>>> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > >>>> + struct phy_configure_opts_mipi_dphy cfg; > >>>> + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > >>>> + int hs_exit, hs_prepare, hs_zero, hs_trail; > >>>> + unsigned long long clock_in_hz = m->clock * 1000; > >>>> > >>>> if (driver_data->has_freqband) > >>>> return; > >>>> > >>>> + /* The dynamic_phy has the ability to adjust PHY Timing settings */ > >>>> + if (driver_data->dynamic_dphy) { > >>>> + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); > >>> This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, > >>> otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: > >>> > >>> ERROR: modpost: "phy_mipi_dphy_get_default_config" > >>> [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! > >>> make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 > >>> make[4]: *** [Makefile:1978: modpost] Error 2 > >>> make[3]: *** [Makefile:357: __build_one_by_one] Error 2 > >>> > >>> I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. > >> That's interesting, I didn't come across that. > >> What did you use for a starting point when you applied the patches? > >> I want to see if I can replicate it. > > next-20230421. My config is pretty much tailored to the Hummingbird Pulse. > > Device drivers for other hardware or things that I can't enable are all > > disabled. For example I don't have PHY_MIXEL_MIPI_DPHY enabled. > > > > Maybe you have some other bridge or phy that selects it enabled? > > > I've observed similar issue while building exynos_defconfig for arm 32bit. Thanks to both of you for the head's up. I'll add a patch to update Kconfig to explicitly select that when I do V3. I was just using the base arm64 'defconfig' option without any customization. adam > > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
On Mon, Apr 24, 2023 at 4:50 AM Adam Ford <aford173@gmail.com> wrote: > > On Mon, Apr 24, 2023 at 4:47 AM Marek Szyprowski > <m.szyprowski@samsung.com> wrote: > > > > On 24.04.2023 11:44, Chen-Yu Tsai wrote: > > > On Mon, Apr 24, 2023 at 5:31 PM Adam Ford <aford173@gmail.com> wrote: > > >> On Mon, Apr 24, 2023 at 1:12 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > >>> On Sun, Apr 23, 2023 at 8:13 PM Adam Ford <aford173@gmail.com> wrote: > > >>>> The DPHY timings are currently hard coded. Since the input > > >>>> clock can be variable, the phy timings need to be variable > > >>>> too. Add an additional variable to the driver data to enable > > >>>> this feature to prevent breaking boards that don't support it. > > >>>> > > >>>> The phy_mipi_dphy_get_default_config function configures the > > >>>> DPHY timings in pico-seconds, and a small macro converts those > > >>>> timings into clock cycles based on the pixel clock rate. > > >>>> > > >>>> Signed-off-by: Adam Ford <aford173@gmail.com> > > >>>> --- > > >>>> drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- > > >>>> include/drm/bridge/samsung-dsim.h | 1 + > > >>>> 2 files changed, 70 insertions(+), 10 deletions(-) > > >>>> > > >>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c > > >>>> index 5b6e7825b92f..f165483d5044 100644 > > >>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c > > >>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c > > >>>> @@ -18,9 +18,7 @@ > > >>>> #include <linux/media-bus-format.h> > > >>>> #include <linux/of_device.h> > > >>>> #include <linux/phy/phy.h> > > >>>> - > > >>>> #include <video/mipi_display.h> > > >>>> - > > >>>> #include <drm/bridge/samsung-dsim.h> > > >>>> #include <drm/drm_panel.h> > > >>>> #include <drm/drm_print.h> > > >>>> @@ -218,6 +216,8 @@ > > >>>> > > >>>> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" > > >>>> > > >>>> +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) > > >>>> + > > >>>> static const char *const clk_names[5] = { > > >>>> "bus_clk", > > >>>> "sclk_mipi", > > >>>> @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { > > >>>> .m_min = 64, > > >>>> .m_max = 1023, > > >>>> .vco_min = 1050, > > >>>> + .dynamic_dphy = 1, > > >>>> }; > > >>>> > > >>>> static const struct samsung_dsim_driver_data * > > >>>> @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) > > >>>> const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; > > >>>> const unsigned int *reg_values = driver_data->reg_values; > > >>>> u32 reg; > > >>>> + struct drm_display_mode *m = &dsi->mode; > > >>>> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > > >>>> + struct phy_configure_opts_mipi_dphy cfg; > > >>>> + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; > > >>>> + int hs_exit, hs_prepare, hs_zero, hs_trail; > > >>>> + unsigned long long clock_in_hz = m->clock * 1000; > > >>>> > > >>>> if (driver_data->has_freqband) > > >>>> return; > > >>>> > > >>>> + /* The dynamic_phy has the ability to adjust PHY Timing settings */ > > >>>> + if (driver_data->dynamic_dphy) { > > >>>> + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); > > >>> This requires adding "select GENERIC_PHY_MIPI_DPHY" to DRM_SAMSUNG_DSIM, > > >>> otherwise with CONFIG_DRM_SAMSUNG_DSIM=m: > > >>> > > >>> ERROR: modpost: "phy_mipi_dphy_get_default_config" > > >>> [drivers/gpu/drm/bridge/samsung-dsim.ko] undefined! > > >>> make[5]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 > > >>> make[4]: *** [Makefile:1978: modpost] Error 2 > > >>> make[3]: *** [Makefile:357: __build_one_by_one] Error 2 > > >>> > > >>> I'm sure there'll be a similar error if CONFIG_DRM_SAMSUNG_DSIM=y. > > >> That's interesting, I didn't come across that. > > >> What did you use for a starting point when you applied the patches? > > >> I want to see if I can replicate it. > > > next-20230421. My config is pretty much tailored to the Hummingbird Pulse. > > > Device drivers for other hardware or things that I can't enable are all > > > disabled. For example I don't have PHY_MIXEL_MIPI_DPHY enabled. > > > > > > Maybe you have some other bridge or phy that selects it enabled? > > > > > > I've observed similar issue while building exynos_defconfig for arm 32bit. > > Thanks to both of you for the head's up. I'll add a patch to update > Kconfig to explicitly select that when I do V3. I was just using the > base arm64 'defconfig' option without any customization. Marek S & Chen-Yu, I added a patch into the series to make this select GENERIC_PHY_MIPI_DPHY, and I tested it with the 32-bit arm config, exynos_defconfig and it now builds successfully. I'm going to wait a few more hours to see if anyone else has any feedback before I send V3. adam > > adam > > > > > > Best regards > > -- > > Marek Szyprowski, PhD > > Samsung R&D Institute Poland > >
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c index 5b6e7825b92f..f165483d5044 100644 --- a/drivers/gpu/drm/bridge/samsung-dsim.c +++ b/drivers/gpu/drm/bridge/samsung-dsim.c @@ -18,9 +18,7 @@ #include <linux/media-bus-format.h> #include <linux/of_device.h> #include <linux/phy/phy.h> - #include <video/mipi_display.h> - #include <drm/bridge/samsung-dsim.h> #include <drm/drm_panel.h> #include <drm/drm_print.h> @@ -218,6 +216,8 @@ #define OLD_SCLK_MIPI_CLK_NAME "pll_clk" +#define PS_TO_CYCLE(PS, MHz) DIV64_U64_ROUND_CLOSEST(((PS) * (MHz)), 1000000000000ULL) + static const char *const clk_names[5] = { "bus_clk", "sclk_mipi", @@ -487,6 +487,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = { .m_min = 64, .m_max = 1023, .vco_min = 1050, + .dynamic_dphy = 1, }; static const struct samsung_dsim_driver_data * @@ -698,13 +699,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) const struct samsung_dsim_driver_data *driver_data = dsi->driver_data; const unsigned int *reg_values = driver_data->reg_values; u32 reg; + struct drm_display_mode *m = &dsi->mode; + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); + struct phy_configure_opts_mipi_dphy cfg; + int clk_prepare, lpx, clk_zero, clk_post, clk_trail; + int hs_exit, hs_prepare, hs_zero, hs_trail; + unsigned long long clock_in_hz = m->clock * 1000; if (driver_data->has_freqband) return; + /* The dynamic_phy has the ability to adjust PHY Timing settings */ + if (driver_data->dynamic_dphy) { + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg); + + /* + * TODO: + * The tech reference manual for i.MX8M Mini/Nano/Plus + * doesn't state what the definition of the PHYTIMING + * bits are beyond their address and bit position. + * After reviewing NXP's downstream code, it appears + * that the various PHYTIMING registers take the number + * of cycles and use various dividers on them. This + * calculation does not result in an exact match to the + * downstream code, but it is very close, and it appears + * to sync at a variety of resolutions. If someone + * can get a more accurate mathematical equation needed + * for these registers, this should be updated. + */ + + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz); + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz); + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz); + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz); + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz); + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz); + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz); + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz); + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz); + } + /* B D-PHY: D-PHY Master & Slave Analog Block control */ reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] | reg_values[PHYCTRL_SLEW_UP]; + samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg); /* @@ -712,7 +750,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) * T HS-EXIT: Time that the transmitter drives LP-11 following a HS * burst */ - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; + if (driver_data->dynamic_dphy) + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit); + else + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT]; + samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg); /* @@ -728,10 +770,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after * the last payload clock bit of a HS transmission burst */ - reg = reg_values[PHYTIMING_CLK_PREPARE] | - reg_values[PHYTIMING_CLK_ZERO] | - reg_values[PHYTIMING_CLK_POST] | - reg_values[PHYTIMING_CLK_TRAIL]; + if (driver_data->dynamic_dphy) { + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) | + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) | + DSIM_PHYTIMING1_CLK_POST(clk_post) | + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail); + } else { + reg = reg_values[PHYTIMING_CLK_PREPARE] | + reg_values[PHYTIMING_CLK_ZERO] | + reg_values[PHYTIMING_CLK_POST] | + reg_values[PHYTIMING_CLK_TRAIL]; + } samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg); @@ -744,8 +793,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi) * T HS-TRAIL: Time that the transmitter drives the flipped differential * state after last payload data bit of a HS transmission burst */ - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] | - reg_values[PHYTIMING_HS_TRAIL]; + + if (driver_data->dynamic_dphy) { + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) | + DSIM_PHYTIMING2_HS_ZERO(hs_zero) | + DSIM_PHYTIMING2_HS_TRAIL(hs_trail); + } else { + reg = reg_values[PHYTIMING_HS_PREPARE] | + reg_values[PHYTIMING_HS_ZERO] | + reg_values[PHYTIMING_HS_TRAIL]; + } + samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg); } @@ -1337,7 +1395,8 @@ static int samsung_dsim_init(struct samsung_dsim *dsi) samsung_dsim_enable_clock(dsi); if (driver_data->wait_for_reset) samsung_dsim_wait_for_reset(dsi); - samsung_dsim_set_phy_ctrl(dsi); + if (!driver_data->has_freqband) + samsung_dsim_set_phy_ctrl(dsi); samsung_dsim_init_link(dsi); dsi->state |= DSIM_STATE_INITIALIZED; diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h index a088d84579bc..25475d78adb3 100644 --- a/include/drm/bridge/samsung-dsim.h +++ b/include/drm/bridge/samsung-dsim.h @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data { u16 m_min; u16 m_max; u64 vco_min; + bool dynamic_dphy; }; struct samsung_dsim_host_ops {
The DPHY timings are currently hard coded. Since the input clock can be variable, the phy timings need to be variable too. Add an additional variable to the driver data to enable this feature to prevent breaking boards that don't support it. The phy_mipi_dphy_get_default_config function configures the DPHY timings in pico-seconds, and a small macro converts those timings into clock cycles based on the pixel clock rate. Signed-off-by: Adam Ford <aford173@gmail.com> --- drivers/gpu/drm/bridge/samsung-dsim.c | 79 +++++++++++++++++++++++---- include/drm/bridge/samsung-dsim.h | 1 + 2 files changed, 70 insertions(+), 10 deletions(-)