| Message ID | 20180125160101.9102-1-philippe.cornu@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/25/2018 05:01 PM, Philippe Cornu wrote: > There is a difference between the panel/bridge requested pixel clock > value and the real one due to the hw platform clock preciseness (pll, > dividers...). This patch updates the adjusted_mode clock value with > the real hw clock value so then attached encoder & connector can use > it for precise timing computations. Reviewed-by: Yannick Fertré <yannick.fertre@st.com> > Signed-off-by: Philippe Cornu <philippe.cornu@st.com> > --- > drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index b48589343ae1..90b3de516c91 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -428,12 +428,35 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc, > reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR); > } > > +static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + struct ltdc_device *ldev = crtc_to_ltdc(crtc); > + int rate = mode->clock * 1000; > + > + /* > + * TODO clk_round_rate() does not work yet. When ready, it can > + * be used instead of clk_set_rate() then clk_get_rate(). > + */ > + > + clk_disable(ldev->pixel_clk); > + if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > + DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > + return false; > + } > + clk_enable(ldev->pixel_clk); > + > + adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; > + > + return true; > +} > + > static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) > { > struct ltdc_device *ldev = crtc_to_ltdc(crtc); > struct drm_display_mode *mode = &crtc->state->adjusted_mode; > struct videomode vm; > - int rate = mode->clock * 1000; > u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; > u32 total_width, total_height; > u32 val; > @@ -456,15 +479,6 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) > total_width = accum_act_w + vm.hfront_porch; > total_height = accum_act_h + vm.vfront_porch; > > - clk_disable(ldev->pixel_clk); > - > - if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > - DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > - return; > - } > - > - clk_enable(ldev->pixel_clk); > - > /* Configures the HS, VS, DE and PC polarities. Default Active Low */ > val = 0; > > @@ -528,6 +542,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc, > } > > static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { > + .mode_fixup = ltdc_crtc_mode_fixup, > .mode_set_nofb = ltdc_crtc_mode_set_nofb, > .atomic_flush = ltdc_crtc_atomic_flush, > .atomic_enable = ltdc_crtc_atomic_enable,
2018-01-29 10:46 GMT+01:00 Yannick FERTRE <yannick.fertre@st.com>: > On 01/25/2018 05:01 PM, Philippe Cornu wrote: >> There is a difference between the panel/bridge requested pixel clock >> value and the real one due to the hw platform clock preciseness (pll, >> dividers...). This patch updates the adjusted_mode clock value with >> the real hw clock value so then attached encoder & connector can use >> it for precise timing computations. > Reviewed-by: Yannick Fertré <yannick.fertre@st.com> >> Signed-off-by: Philippe Cornu <philippe.cornu@st.com> Applied on drm-misc-next. Regards, Benjamin >> --- >> drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++---------- >> 1 file changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c >> index b48589343ae1..90b3de516c91 100644 >> --- a/drivers/gpu/drm/stm/ltdc.c >> +++ b/drivers/gpu/drm/stm/ltdc.c >> @@ -428,12 +428,35 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc, >> reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR); >> } >> >> +static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode, >> + struct drm_display_mode *adjusted_mode) >> +{ >> + struct ltdc_device *ldev = crtc_to_ltdc(crtc); >> + int rate = mode->clock * 1000; >> + >> + /* >> + * TODO clk_round_rate() does not work yet. When ready, it can >> + * be used instead of clk_set_rate() then clk_get_rate(). >> + */ >> + >> + clk_disable(ldev->pixel_clk); >> + if (clk_set_rate(ldev->pixel_clk, rate) < 0) { >> + DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); >> + return false; >> + } >> + clk_enable(ldev->pixel_clk); >> + >> + adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; >> + >> + return true; >> +} >> + >> static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) >> { >> struct ltdc_device *ldev = crtc_to_ltdc(crtc); >> struct drm_display_mode *mode = &crtc->state->adjusted_mode; >> struct videomode vm; >> - int rate = mode->clock * 1000; >> u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; >> u32 total_width, total_height; >> u32 val; >> @@ -456,15 +479,6 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) >> total_width = accum_act_w + vm.hfront_porch; >> total_height = accum_act_h + vm.vfront_porch; >> >> - clk_disable(ldev->pixel_clk); >> - >> - if (clk_set_rate(ldev->pixel_clk, rate) < 0) { >> - DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); >> - return; >> - } >> - >> - clk_enable(ldev->pixel_clk); >> - >> /* Configures the HS, VS, DE and PC polarities. Default Active Low */ >> val = 0; >> >> @@ -528,6 +542,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc, >> } >> >> static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { >> + .mode_fixup = ltdc_crtc_mode_fixup, >> .mode_set_nofb = ltdc_crtc_mode_set_nofb, >> .atomic_flush = ltdc_crtc_atomic_flush, >> .atomic_enable = ltdc_crtc_atomic_enable,
Hi Philippe, On Thursday, 25 January 2018 18:01:01 EET Philippe Cornu wrote: > There is a difference between the panel/bridge requested pixel clock > value and the real one due to the hw platform clock preciseness (pll, > dividers...). This patch updates the adjusted_mode clock value with > the real hw clock value so then attached encoder & connector can use > it for precise timing computations. > > Signed-off-by: Philippe Cornu <philippe.cornu@st.com> > --- > drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index b48589343ae1..90b3de516c91 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -428,12 +428,35 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc > *crtc, reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR); > } > > +static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, > + const struct drm_display_mode *mode, > + struct drm_display_mode *adjusted_mode) > +{ > + struct ltdc_device *ldev = crtc_to_ltdc(crtc); > + int rate = mode->clock * 1000; > + > + /* > + * TODO clk_round_rate() does not work yet. When ready, it can > + * be used instead of clk_set_rate() then clk_get_rate(). > + */ Why does it fail ? Is it due to the STM clock source implementation ? This looks like a big hack, I'd rather see clk_round_rate() being fixed. > + clk_disable(ldev->pixel_clk); > + if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > + DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > + return false; > + } > + clk_enable(ldev->pixel_clk); > + > + adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; > + > + return true; > +} > + > static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) > { > struct ltdc_device *ldev = crtc_to_ltdc(crtc); > struct drm_display_mode *mode = &crtc->state->adjusted_mode; > struct videomode vm; > - int rate = mode->clock * 1000; > u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; > u32 total_width, total_height; > u32 val; > @@ -456,15 +479,6 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc > *crtc) total_width = accum_act_w + vm.hfront_porch; > total_height = accum_act_h + vm.vfront_porch; > > - clk_disable(ldev->pixel_clk); > - > - if (clk_set_rate(ldev->pixel_clk, rate) < 0) { > - DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); > - return; > - } > - > - clk_enable(ldev->pixel_clk); > - > /* Configures the HS, VS, DE and PC polarities. Default Active Low */ > val = 0; > > @@ -528,6 +542,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc > *crtc, } > > static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { > + .mode_fixup = ltdc_crtc_mode_fixup, > .mode_set_nofb = ltdc_crtc_mode_set_nofb, > .atomic_flush = ltdc_crtc_atomic_flush, > .atomic_enable = ltdc_crtc_atomic_enable,
2018-01-29 11:43 GMT+01:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Philippe, > > On Thursday, 25 January 2018 18:01:01 EET Philippe Cornu wrote: >> There is a difference between the panel/bridge requested pixel clock >> value and the real one due to the hw platform clock preciseness (pll, >> dividers...). This patch updates the adjusted_mode clock value with >> the real hw clock value so then attached encoder & connector can use >> it for precise timing computations. >> >> Signed-off-by: Philippe Cornu <philippe.cornu@st.com> >> --- >> drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++---------- >> 1 file changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c >> index b48589343ae1..90b3de516c91 100644 >> --- a/drivers/gpu/drm/stm/ltdc.c >> +++ b/drivers/gpu/drm/stm/ltdc.c >> @@ -428,12 +428,35 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc >> *crtc, reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR); >> } >> >> +static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, >> + const struct drm_display_mode *mode, >> + struct drm_display_mode *adjusted_mode) >> +{ >> + struct ltdc_device *ldev = crtc_to_ltdc(crtc); >> + int rate = mode->clock * 1000; >> + >> + /* >> + * TODO clk_round_rate() does not work yet. When ready, it can >> + * be used instead of clk_set_rate() then clk_get_rate(). >> + */ > > Why does it fail ? Is it due to the STM clock source implementation ? This > looks like a big hack, I'd rather see clk_round_rate() being fixed. We have log the point and we will investigate it asap. Regards, Benjamin > >> + clk_disable(ldev->pixel_clk); >> + if (clk_set_rate(ldev->pixel_clk, rate) < 0) { >> + DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); >> + return false; >> + } >> + clk_enable(ldev->pixel_clk); >> + >> + adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; >> + >> + return true; >> +} >> + >> static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) >> { >> struct ltdc_device *ldev = crtc_to_ltdc(crtc); >> struct drm_display_mode *mode = &crtc->state->adjusted_mode; >> struct videomode vm; >> - int rate = mode->clock * 1000; >> u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; >> u32 total_width, total_height; >> u32 val; >> @@ -456,15 +479,6 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc >> *crtc) total_width = accum_act_w + vm.hfront_porch; >> total_height = accum_act_h + vm.vfront_porch; >> >> - clk_disable(ldev->pixel_clk); >> - >> - if (clk_set_rate(ldev->pixel_clk, rate) < 0) { >> - DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); >> - return; >> - } >> - >> - clk_enable(ldev->pixel_clk); >> - >> /* Configures the HS, VS, DE and PC polarities. Default Active Low */ >> val = 0; >> >> @@ -528,6 +542,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc >> *crtc, } >> >> static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { >> + .mode_fixup = ltdc_crtc_mode_fixup, >> .mode_set_nofb = ltdc_crtc_mode_set_nofb, >> .atomic_flush = ltdc_crtc_atomic_flush, >> .atomic_enable = ltdc_crtc_atomic_enable, > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index b48589343ae1..90b3de516c91 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -428,12 +428,35 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc *crtc, reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR); } +static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc, + const struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) +{ + struct ltdc_device *ldev = crtc_to_ltdc(crtc); + int rate = mode->clock * 1000; + + /* + * TODO clk_round_rate() does not work yet. When ready, it can + * be used instead of clk_set_rate() then clk_get_rate(). + */ + + clk_disable(ldev->pixel_clk); + if (clk_set_rate(ldev->pixel_clk, rate) < 0) { + DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); + return false; + } + clk_enable(ldev->pixel_clk); + + adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000; + + return true; +} + static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) { struct ltdc_device *ldev = crtc_to_ltdc(crtc); struct drm_display_mode *mode = &crtc->state->adjusted_mode; struct videomode vm; - int rate = mode->clock * 1000; u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h; u32 total_width, total_height; u32 val; @@ -456,15 +479,6 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc) total_width = accum_act_w + vm.hfront_porch; total_height = accum_act_h + vm.vfront_porch; - clk_disable(ldev->pixel_clk); - - if (clk_set_rate(ldev->pixel_clk, rate) < 0) { - DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate); - return; - } - - clk_enable(ldev->pixel_clk); - /* Configures the HS, VS, DE and PC polarities. Default Active Low */ val = 0; @@ -528,6 +542,7 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc, } static const struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { + .mode_fixup = ltdc_crtc_mode_fixup, .mode_set_nofb = ltdc_crtc_mode_set_nofb, .atomic_flush = ltdc_crtc_atomic_flush, .atomic_enable = ltdc_crtc_atomic_enable,
There is a difference between the panel/bridge requested pixel clock value and the real one due to the hw platform clock preciseness (pll, dividers...). This patch updates the adjusted_mode clock value with the real hw clock value so then attached encoder & connector can use it for precise timing computations. Signed-off-by: Philippe Cornu <philippe.cornu@st.com> --- drivers/gpu/drm/stm/ltdc.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-)