Hi Tomi, Thank you for the patch. On Fri, Apr 17, 2020 at 02:41:51PM +0300, Tomi Valkeinen wrote: > If the given videomode does not specify DISPLAY_FLAG_* for the specific > signal property, the driver used a default value. These defaults were > never thought through, as the expectation was that all the DISPLAY_FLAGS > are always set explicitly. > > With DRM bridge and panel drivers this is not the case, and while that > issue should be resolved in the future, it's still good to have sane > signal defaults. > > This patch changes the defaults to what the hardware has as reset > defaults. Also, based on my experience, I think they make sense and are > more likely correct than the defaults without this patch. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > --- > drivers/gpu/drm/omapdrm/dss/dispc.c | 33 ++++++----------------------- > 1 file changed, 6 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c > index dbb90f2d2ccd..6639ee9b05d3 100644 > --- a/drivers/gpu/drm/omapdrm/dss/dispc.c > +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c > @@ -3137,33 +3137,12 @@ static void _dispc_mgr_set_lcd_timings(struct dispc_device *dispc, > dispc_write_reg(dispc, DISPC_TIMING_H(channel), timing_h); > dispc_write_reg(dispc, DISPC_TIMING_V(channel), timing_v); > > - if (vm->flags & DISPLAY_FLAGS_VSYNC_HIGH) > - vs = false; > - else > - vs = true; > - > - if (vm->flags & DISPLAY_FLAGS_HSYNC_HIGH) > - hs = false; > - else > - hs = true; > - > - if (vm->flags & DISPLAY_FLAGS_DE_HIGH) > - de = false; > - else > - de = true; > - > - if (vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE) > - ipc = false; > - else > - ipc = true; > - > - /* always use the 'rf' setting */ > - onoff = true; > - > - if (vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE) > - rf = true; > - else > - rf = false; > + vs = !!(vm->flags & DISPLAY_FLAGS_VSYNC_LOW); > + hs = !!(vm->flags & DISPLAY_FLAGS_HSYNC_LOW); > + de = !!(vm->flags & DISPLAY_FLAGS_DE_LOW); > + ipc = !!(vm->flags & DISPLAY_FLAGS_PIXDATA_NEGEDGE); > + onoff = true; /* always use the 'rf' setting */ > + rf = !!(vm->flags & DISPLAY_FLAGS_SYNC_POSEDGE); Would it make sense to WARN() if flags are not set, to catch offenders and fix them ? Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > l = FLD_VAL(onoff, 17, 17) | > FLD_VAL(rf, 16, 16) | -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
