On Thu, Apr 19, 2018 at 9:34 PM, Ondřej Jirman <doudahwezomiechahtah@xxxxxx> wrote: > Hello Giulio, > > this patch breaks LVDS output on A83T. Without it, modesetting works, > with it there's no output. > > Some more info below... > > On Tue, Mar 13, 2018 at 12:20:19PM +0100, Giulio Benetti wrote: >> mode_valid function is missing for lvds. >> >> Add it making it pointed by encoder helper functions. >> >> Signed-off-by: Giulio Benetti <giulio.benetti@xxxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/sun4i/sun4i_lvds.c | 55 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 55 insertions(+) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_lvds.c b/drivers/gpu/drm/sun4i/sun4i_lvds.c >> index be3f14d..bffff4c 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_lvds.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_lvds.c >> @@ -94,9 +94,64 @@ static void sun4i_lvds_encoder_disable(struct drm_encoder *encoder) >> } >> } >> >> +static enum drm_mode_status sun4i_lvds_encoder_mode_valid(struct drm_encoder *crtc, >> + const struct drm_display_mode *mode) >> +{ >> + struct sun4i_lvds *lvds = drm_encoder_to_sun4i_lvds(crtc); >> + struct sun4i_tcon *tcon = lvds->tcon; >> + u32 hsync = mode->hsync_end - mode->hsync_start; >> + u32 vsync = mode->vsync_end - mode->vsync_start; >> + unsigned long rate = mode->clock * 1000; >> + long rounded_rate; >> + >> + DRM_DEBUG_DRIVER("Validating modes...\n"); >> + >> + if (hsync < 1) >> + return MODE_HSYNC_NARROW; >> + >> + if (hsync > 0x3ff) >> + return MODE_HSYNC_WIDE; >> + >> + if ((mode->hdisplay < 1) || (mode->htotal < 1)) >> + return MODE_H_ILLEGAL; >> + >> + if ((mode->hdisplay > 0x7ff) || (mode->htotal > 0xfff)) >> + return MODE_BAD_HVALUE; >> + >> + DRM_DEBUG_DRIVER("Horizontal parameters OK\n"); >> + >> + if (vsync < 1) >> + return MODE_VSYNC_NARROW; >> + >> + if (vsync > 0x3ff) >> + return MODE_VSYNC_WIDE; >> + >> + if ((mode->vdisplay < 1) || (mode->vtotal < 1)) >> + return MODE_V_ILLEGAL; >> + >> + if ((mode->vdisplay > 0x7ff) || (mode->vtotal > 0xfff)) >> + return MODE_BAD_VVALUE; >> + >> + DRM_DEBUG_DRIVER("Vertical parameters OK\n"); >> + >> + tcon->dclk_min_div = 7; >> + tcon->dclk_max_div = 7; > > Why would validation function change any state anywhere? > >> + rounded_rate = clk_round_rate(tcon->dclk, rate); > > The issue is here, on A83T TBS A711 tablet, I get... > > sun4i-tcon 1c0c000.lcd-controller: XXX: hsync=20 hdisplay=1024 htotal=1384 > vsync=5 vdisplay=600 vtotal=640 rate=52000000 rounded_rate=51857142 > >> + if (rounded_rate < rate) >> + return MODE_CLOCK_LOW; >> + >> + if (rounded_rate > rate) >> + return MODE_CLOCK_HIGH; > > ... while the previous conditions require an exact match for some reason. > > But HW works fine without an exact rate match. Why is exact match required here? This thread might provide some more info, though we could never get an agreement. https://patchwork.kernel.org/patch/9446385/ ChenYu > > thank you, > Ondrej > >> + DRM_DEBUG_DRIVER("Clock rate OK\n"); >> + >> + return MODE_OK; >> +} >> + >> static const struct drm_encoder_helper_funcs sun4i_lvds_enc_helper_funcs = { >> .disable = sun4i_lvds_encoder_disable, >> .enable = sun4i_lvds_encoder_enable, >> + .mode_valid = sun4i_lvds_encoder_mode_valid, >> }; >> >> static const struct drm_encoder_funcs sun4i_lvds_enc_funcs = { _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel