Re: [PATCH 22/29] drm/omap: Move DISPC timing checks to CRTC .mode_valid() operation

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@ideasonboard.com>
> >>>> ---
> >>> 
> >>> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> >>> 
> >>> 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@collabora.com>

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel