Re: [Intel-gfx] [PATCH 1/5] drm: Add atomic helper to redo a modeset on current mode

From: Sean Paul <seanpaul@chromium.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Manasi Navare <manasi.d.navare@intel.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 1/5] drm: Add atomic helper to redo a modeset on current mode
Date: Mon, 24 Oct 2016 14:38:17 -0400	[thread overview]
Message-ID: <CAOw6vbLXjOCRrPBvtYtthLFQAGb9g5ejtmAqKyrd0yPCkh2GPQ@mail.gmail.com> (raw)
In-Reply-To: <CAKMK7uEQL2eX6FYojJcF1fFv=Y6MLGR6z6XnVrBUhHgbZtERDQ@mail.gmail.com>

On Mon, Oct 24, 2016 at 3:12 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Oct 24, 2016 at 9:00 AM, Manasi Navare
> <manasi.d.navare@intel.com> wrote:
>>> I guess we just need to do some additional work on top to make sure the
>>> vblank ioctl can't see invalid state. Which would then again make upfront
>>> link trainig possible ...
>>
>> Could you share some more details on what work would need to be done
>> for vblank? Then I can investigate it a little bit more tomor during the day.
>
> So the trouble is that drm_irq.c is still from the old pre-kms days.
> Which means it deals in int pipe instead of struct drm_crtc *, among
> other things. But we can't simply switch over because some old drivers
> (pre-kms ones) still depend upon that old code. Hence we need:
>
> 1. Duplicate most of the code in drm_irq.c into a new
> drm_crtc_vblank.c, which is only used for kms drivers.

Why duplicate all of this code? That seems a bit wasteful.

Can't we just add the inverse of drm_crtc_pipe() (coincidentally, I
also suggested we introduce this function in "drm: zte: add initial
vou drm driver"), and use those to map between pipe and crtc where
appropriate? The crtc locks could be acquired/dropped based on
DRIVER_MODESET in drm_irq.c

Unrelated to the vblank discussion: This functionality is pretty
similar to what happens on suspend/resume. Any chance we can share?

Sean

> And in the
> ioctl code have a DRIVER_MODESET check and either call the new kms
> version (in drm_crtc_vblank.c) or the old one (still in drm_irq.c).
>
> 2. Convert the int pipe into a drm_crtc pointer in the new kms code.
> For prettiness we could (try) to do that as much as possible.
>
> 3. Grab the drm_crtc->mutex in the ioctl code to block these races.
>
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel