Hi Sebastian, (CC'ing Daniel) On Tuesday, 11 December 2018 03:07:45 EET Sebastian Reichel wrote: > On Mon, Dec 10, 2018 at 02:14:01PM +0100, Sebastian Reichel wrote: > > On Mon, Dec 10, 2018 at 09:50:08AM +0200, Laurent Pinchart wrote: > >> On Monday, 10 December 2018 00:07:55 EET Sebastian Reichel wrote: > >>> On Wed, Dec 05, 2018 at 05:00:15PM +0200, Laurent Pinchart wrote: > >>>> The DISPC timings checks relate to the CRTC, but they're performed > >>>> in the encoder and connector .atomic_check() and .mode_valid() > >>>> operations. Move them to the CRTC .mode_valid() operation. > >>>> > >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > >>>> --- > >>> > >>> Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > >>> > >>> This should also fix the issue with DSI in a less ugly way: > >>> > >>> https://lists.freedesktop.org/archives/dri-devel/2018-November/196622. > >>> html > >> > >> Should I add the > >> > >> Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation > >> directly") > >> > >> tag to this patch ? > > > > I plan to test this series in combination with my DSI patches today > > or tomorrow evening. If it works, probably the following should be > > added to the patch description: > > > > -------------------- > > As a side-effect this fixes DSI, which uses slightly different mode > > configuration for the DSI encoder and the DISPC. > > > > Fixes: 7c27fa57ef31 ("drm/omap: Call dispc timings check operation > > directly") > > -------------------- > > I merged this series with my remaining DSI command mode patches > (btw. this series does not apply cleanly to master due to a recent > patch from Tomi). I'll fix that, thank you for the notice. > Unfortunately this patch does not fix the issue. After this patch the > mgr_check_timings() call in omap_crtc_mode_valid() will fail. > > The good news: The Droid 4 display worked again after dropping the > mgr_check_timings call as a quick hack. So the other patches from > this set are > > Tested-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> Thank you for testing this. > The problem is, that DISPC timings for DSI are not directly > calculated from mode. Instead DSI code does some modifications > before applying the DISPC settings. I wonder what the best way to fix this would be. DSI isn't so special in this regard, any encoder can modify modes, resulting in the CRTC outputting a mode significantly different than the mode being displayed. This is handled by the DRM helpers through the .mode_fixup() operations, which can mangle modes along the pipeline. At atomic commit time the atomic helpers call .mode_valid() first, followed by .mode_fixup(). An easy fix would be to remove our .mode_valid() handlers completely and perform both fixup and validation in the .mode_fixup() handlers. However, the .mode_valid() operations are also used in drm_helper_probe_single_connector_modes(), where no mode fixup is performed. We can't drop .mode_valid() for that code path (at least not as anything else than a hack). Daniel, what's the expected way to handle this ? -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel