Hi Tomi, On Fri, Apr 17, 2020 at 04:34:19PM +0300, Tomi Valkeinen wrote: > On 17/04/2020 16:29, Laurent Pinchart wrote: > > 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, > > Maybe at some point, but for now it would probably be printing WARNs on all boards. I haven't looked > at exactly which driver combinations get the bus flags/formats right, but I have a feeling that it's > not too many. Fair enough. Maybe we should try it locally first and see how many components are faulty. > And some pieces of hardware also may be "don't care" for certain flags, so I think it's a valid case > that a bridge/panel doesn't define some of the flags. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
