Messages in this thread | | | Subject | Re: [v2] drm/sun4i: add lvds mode_valid function | From | Giulio Benetti <> | Date | Thu, 19 Apr 2018 16:02:08 +0200 |
| |
Hi everybody,
Il 19/04/2018 15:36, Chen-Yu Tsai ha scritto: > On Thu, Apr 19, 2018 at 9:34 PM, Ondřej Jirman > <doudahwezomiechahtah@xff.cz> 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@micronovasrl.com> >>> --- >>> 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/
Thanks for pointing that Thread ChenYu. So the only chance is to trim frequency according to encoder capabilities. I agree to block when encoder can't provide frequency specified, otherwise you could drive you panel at the near the lowest(highest) frequency and get out of limits for a few without knowing it.
Best regards
-- Giulio Benetti CTO
MICRONOVA SRL Sede: Via A. Niedda 3 - 35010 Vigonza (PD) Tel. 049/8931563 - Fax 049/8931346 Cod.Fiscale - P.IVA 02663420285 Capitale Sociale € 26.000 i.v. Iscritta al Reg. Imprese di Padova N. 02663420285 Numero R.E.A. 258642
> > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
| |