drm_framebuffer_cleanup cleanup..

[Rob Clark]

On Fri, Dec 9, 2011 at 11:44 AM, Dave Airlie <airlied at gmail.com> wrote:
> On Fri, Dec 9, 2011 at 5:40 PM, Rob Clark <rob.clark at linaro.org> wrote:
>> On Wed, Oct 19, 2011 at 8:27 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>>>> +static void omap_framebuffer_destroy(struct drm_framebuffer *fb)
>>>> +{
>>>> +     struct drm_device *dev = fb->dev;
>>>> +     struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
>>>> +
>>>> +     DBG("destroy: FB ID: %d (%p)", fb->base.id, fb);
>>>> +
>>>> +     drm_framebuffer_cleanup(fb);
>>>
>>> This is a bit a general mess in kms. All drivers need to call this, and
>>> for added hilarity
>>> - drm_crtc.c drm_mode_rmfb has a TODO that this is missing
>>> - nouveau/radeon only do this _after_ unref'ing the backing storage
>>> - gma500 also tries to restore the kernel console here which should be
>>>  done in lastclose (because you might disable another userspace fb on
>>>  another output).
>>>
>>> Can I prod you to write the patches to clean this up and move
>>> drm_framebuffer_cleanup into common code?
>>
>> fyi, I'm starting to look at this.. looks like crtc/encoder/connector
>> cleanup could also benefit from some similar treatment.  I'll start w/
>> just fb, since that is a resource that is actually dynamically
>> created/destroyed, so seems a bit more critical..
>
> I'm not sure reversing the flow here is the right answer, this is
> mainly trying to avoid midlayering things, the driver controls when
> stuff happens and it controls init the framebuffer object it controls
> destroying it as well.

ok.. leaving the drm_xyz_cleanup() call in the driver_xyz_destroy()
fxn, rather than calling the driver destroy fxn from
drm_xyz_cleanup(), would be a much smaller change.  It does give the
driver a bit more rope to hang itself, and I guess at least the
drm_framebuffer_cleanup() should happen before backing store gets
unref'd so that all the CRTCs are turned off.

BR,
-R


> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel