Message ID | 1490706496-4959-28-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Tomi, Thank you for the patch. On Tuesday 28 Mar 2017 16:08:13 Tomi Valkeinen wrote: > Instead of printing 0/1 for display flags like vsync high/low, use a > tri-state print (-1/0/1) to indicate the "undefined" state. Even better would be to use high/low/unknown (or H/L/U or any similar set of values). > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > drivers/gpu/drm/omapdrm/dss/dispc.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c > b/drivers/gpu/drm/omapdrm/dss/dispc.c index 83241052df6b..2e6a71dbc25d > 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > @@ -3229,6 +3229,16 @@ static void _dispc_mgr_set_lcd_timings(enum > omap_channel channel, } > } > > +static int vm_flag_to_int(enum display_flags flags, enum display_flags > high, + enum display_flags low) > +{ > + if (flags & high) > + return 1; > + if (flags & low) > + return -1; > + return 0; > +} > + > /* change name to mode? */ > static void dispc_mgr_set_timings(enum omap_channel channel, > const struct videomode *vm) > @@ -3258,11 +3268,11 @@ static void dispc_mgr_set_timings(enum omap_channel > channel, t.hsync_len, t.hfront_porch, t.hback_porch, > t.vsync_len, t.vfront_porch, t.vback_porch); > DSSDBG("vsync_level %d hsync_level %d data_pclk_edge %d de_level %d > sync_pclk_edge %d\n", - !!(t.flags & DISPLAY_FLAGS_VSYNC_HIGH), > - !!(t.flags & DISPLAY_FLAGS_HSYNC_HIGH), > - !!(t.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE), > - !!(t.flags & DISPLAY_FLAGS_DE_HIGH), > - !!(t.flags & DISPLAY_FLAGS_SYNC_POSEDGE)); > + vm_flag_to_int(t.flags, DISPLAY_FLAGS_VSYNC_HIGH, > DISPLAY_FLAGS_VSYNC_LOW), + vm_flag_to_int(t.flags, > DISPLAY_FLAGS_HSYNC_HIGH, DISPLAY_FLAGS_HSYNC_LOW), > + vm_flag_to_int(t.flags, DISPLAY_FLAGS_PIXDATA_POSEDGE, > DISPLAY_FLAGS_PIXDATA_NEGEDGE), + vm_flag_to_int(t.flags, > DISPLAY_FLAGS_DE_HIGH, DISPLAY_FLAGS_DE_LOW), + vm_flag_to_int(t.flags, > DISPLAY_FLAGS_SYNC_POSEDGE, DISPLAY_FLAGS_SYNC_NEGEDGE)); > > DSSDBG("hsync %luHz, vsync %luHz\n", ht, vt); > } else {
On 29/03/17 12:00, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tuesday 28 Mar 2017 16:08:13 Tomi Valkeinen wrote: >> Instead of printing 0/1 for display flags like vsync high/low, use a >> tri-state print (-1/0/1) to indicate the "undefined" state. > > Even better would be to use high/low/unknown (or H/L/U or any similar set of > values). Hmm I don't know... high/low/unknown would make the debug print much longer, whereas H/L/U doesn't mean anything unless you check it from the driver code. Well, probably -1/0/1 is not exactly clear either, but it's the shortest and clearest option I have in mind. Tomi
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c index 83241052df6b..2e6a71dbc25d 100644 --- a/drivers/gpu/drm/omapdrm/dss/dispc.c +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c @@ -3229,6 +3229,16 @@ static void _dispc_mgr_set_lcd_timings(enum omap_channel channel, } } +static int vm_flag_to_int(enum display_flags flags, enum display_flags high, + enum display_flags low) +{ + if (flags & high) + return 1; + if (flags & low) + return -1; + return 0; +} + /* change name to mode? */ static void dispc_mgr_set_timings(enum omap_channel channel, const struct videomode *vm) @@ -3258,11 +3268,11 @@ static void dispc_mgr_set_timings(enum omap_channel channel, t.hsync_len, t.hfront_porch, t.hback_porch, t.vsync_len, t.vfront_porch, t.vback_porch); DSSDBG("vsync_level %d hsync_level %d data_pclk_edge %d de_level %d sync_pclk_edge %d\n", - !!(t.flags & DISPLAY_FLAGS_VSYNC_HIGH), - !!(t.flags & DISPLAY_FLAGS_HSYNC_HIGH), - !!(t.flags & DISPLAY_FLAGS_PIXDATA_POSEDGE), - !!(t.flags & DISPLAY_FLAGS_DE_HIGH), - !!(t.flags & DISPLAY_FLAGS_SYNC_POSEDGE)); + vm_flag_to_int(t.flags, DISPLAY_FLAGS_VSYNC_HIGH, DISPLAY_FLAGS_VSYNC_LOW), + vm_flag_to_int(t.flags, DISPLAY_FLAGS_HSYNC_HIGH, DISPLAY_FLAGS_HSYNC_LOW), + vm_flag_to_int(t.flags, DISPLAY_FLAGS_PIXDATA_POSEDGE, DISPLAY_FLAGS_PIXDATA_NEGEDGE), + vm_flag_to_int(t.flags, DISPLAY_FLAGS_DE_HIGH, DISPLAY_FLAGS_DE_LOW), + vm_flag_to_int(t.flags, DISPLAY_FLAGS_SYNC_POSEDGE, DISPLAY_FLAGS_SYNC_NEGEDGE)); DSSDBG("hsync %luHz, vsync %luHz\n", ht, vt); } else {
Instead of printing 0/1 for display flags like vsync high/low, use a tri-state print (-1/0/1) to indicate the "undefined" state. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/dss/dispc.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-)