Hi On Wed, Dec 18, 2013 at 8:42 AM, Takashi Iwai <tiwai@xxxxxxx> wrote: > Hi, > > with the recent enablement of simplefb on x86, cirrusdrmfb on QEMU/KVM > gets broken now, as reported at: > https://bugzilla.novell.com/show_bug.cgi?id=855821 > > The cirrus VGA resource is reserved at first as "BOOTFB" in > arch/x86/kernel/sysfb_simplefb.c, which is taken by simplefb platform > device. This resource is, however, never released until the platform > device is destroyed, and the framebuffer switching doesn't trigger > it. It calls fb's destroy callback, at most. Then, cirrus driver > tries to assign the resource, fails and gives up, resulting in a > complete blank screen. > > The same problem should exist on other KMS drivers like mgag200 or > ast, not only cirrus. Intel graphics doesn't hit this problem just > because the reserved iomem by BOOTFB isn't required by i915 driver. > > The patch below is a quick attempt to solve the issue. It adds a new > API function for releasing resources of platform_device, and call it > in destroy op of simplefb. But, forcibly releasing resources of a > parent device doesn't sound like a correct design. We may take such > as a band aid, but definitely need a more fundamental fix. > > Any thoughts? That bug always existed, simplefb is just the first driver to hit it (vesafb/efifb didn't use resources). I'm aware of the issue but as a workaround you can simply disable CONFIG_X86_SYSFB. That restores the old behavior. As a proper fix, I'd propose something like: dev = platform_find_device("platform-framebuffer"); platform_remove_device(dev); And we wrap this as: sysfb_remove_framebuffers() in arch/x86/kernel/sysfb.c This should cause a ->remove() event for the platform-driver and correctly release the resources. Comments? I will try to write a patch and send it later. Thanks David > > thanks, > > Takashi > > --- > diff --git a/drivers/base/platform.c b/drivers/base/platform.c > index 3a94b79..f939236 100644 > --- a/drivers/base/platform.c > +++ b/drivers/base/platform.c > @@ -267,6 +267,23 @@ int platform_device_add_data(struct platform_device *pdev, const void *data, > } > EXPORT_SYMBOL_GPL(platform_device_add_data); > > +static void do_release_resources(struct platform_device *pdev, int nums) > +{ > + int i; > + > + for (i = 0; i < nums; i++) { > + struct resource *r = &pdev->resource[i]; > + unsigned long type = resource_type(r); > + > + if (type == IORESOURCE_MEM || type == IORESOURCE_IO) > + release_resource(r); > + } > + > + kfree(pdev->resource); > + pdev->resource = NULL; > + pdev->num_resources = 0; > +} > + > /** > * platform_device_add - add a platform device to device hierarchy > * @pdev: platform device we're adding > @@ -342,13 +359,7 @@ int platform_device_add(struct platform_device *pdev) > pdev->id = PLATFORM_DEVID_AUTO; > } > > - while (--i >= 0) { > - struct resource *r = &pdev->resource[i]; > - unsigned long type = resource_type(r); > - > - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) > - release_resource(r); > - } > + do_release_resources(pdev, i - 1); > > err_out: > return ret; > @@ -365,8 +376,6 @@ EXPORT_SYMBOL_GPL(platform_device_add); > */ > void platform_device_del(struct platform_device *pdev) > { > - int i; > - > if (pdev) { > device_del(&pdev->dev); > > @@ -375,17 +384,17 @@ void platform_device_del(struct platform_device *pdev) > pdev->id = PLATFORM_DEVID_AUTO; > } > > - for (i = 0; i < pdev->num_resources; i++) { > - struct resource *r = &pdev->resource[i]; > - unsigned long type = resource_type(r); > - > - if (type == IORESOURCE_MEM || type == IORESOURCE_IO) > - release_resource(r); > - } > + do_release_resources(pdev, pdev->num_resources); > } > } > EXPORT_SYMBOL_GPL(platform_device_del); > > +void platform_device_release_resources(struct platform_device *pdev) > +{ > + do_release_resources(pdev, pdev->num_resources); > +} > +EXPORT_SYMBOL_GPL(platform_device_release_resources); > + > /** > * platform_device_register - add a platform-level device > * @pdev: platform device we're adding > diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c > index 210f3a0..fbf5e89 100644 > --- a/drivers/video/simplefb.c > +++ b/drivers/video/simplefb.c > @@ -70,6 +70,7 @@ static void simplefb_destroy(struct fb_info *info) > { > if (info->screen_base) > iounmap(info->screen_base); > + platform_device_release_resources(to_platform_device(info->device)); > } > > static struct fb_ops simplefb_ops = { > diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h > index 16f6654..7cc1f54 100644 > --- a/include/linux/platform_device.h > +++ b/include/linux/platform_device.h > @@ -42,6 +42,7 @@ struct platform_device { > > extern int platform_device_register(struct platform_device *); > extern void platform_device_unregister(struct platform_device *); > +extern void platform_device_release_resources(struct platform_device *pdev); > > extern struct bus_type platform_bus_type; > extern struct device platform_bus; -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
