Hi Am 29.06.20 um 11:06 schrieb Daniel Vetter:
+ ARRAY_SIZE(simplekms_formats), + simplekms_format_modifiers, + connector); + if (ret) + return ret; + + drm_mode_config_reset(dev);This breaks fastboot. I think ideally we'd have the state represent everything is on, and allocate an fb + buffer with the current contents of the framebuffer. Since we can allocate an fb that matches this shouldn't be a problem, just a raw memcpy_fromio should do the job. Having a nice new simplekms drm driver and then losing fastboot feels like slightly off tradeoff. Maybe in a follow-up patch, but before fbcon setup? Since ideally fbcon also takes over the already existing framebuffer we allocated, so that as long as nothing clears the fb (i.e. fbcon is quiet) we'd preserve the original framebuffer throughout the boot-up sequence.
I recently looked at how to implement this and it seems fairly complicated.What we want it to adopt the current mode config into fbcon (and probably other in-kernel clients). The kernel client code uses it's own file instance to allocate the framebuffer objects against. So we cannot read-out the framebuffer state here. We'd ideally do this in the fbdev code.
I read through the proposal for read-out helpers. i915 seems to have lots of special cases. Can we adopt a simplified version that is just good enough to get the initial state for fbdev?
Best regards Thomas
+ + return 0; +} + +/* + * Init / Cleanup + */ + +static void simplekms_device_cleanup(struct simplekms_device* sdev) +{ + struct drm_device *dev = &sdev->dev; + + drm_dev_unregister(dev);I'd inline this, I guess there was once more before you switched everything over to devm_+} + +static struct simplekms_device * +simplekms_device_create(struct drm_driver *drv, struct platform_device *pdev) +{ + struct simplekms_device *sdev; + int ret; + + sdev = devm_drm_dev_alloc(&pdev->dev, drv, struct simplekms_device, + dev); + if (IS_ERR(sdev)) + return ERR_CAST(sdev); + sdev->pdev = pdev; + + ret = simplekms_device_init_fb(sdev); + if (ret) + return ERR_PTR(ret); + ret = simplekms_device_init_mm(sdev); + if (ret) + return ERR_PTR(ret); + ret = simplekms_device_init_modeset(sdev); + if (ret) + return ERR_PTR(ret); + + return sdev; +} + +/* + * DRM driver + */ + +DEFINE_DRM_GEM_FOPS(simplekms_fops); + +static struct drm_driver simplekms_driver = { + DRM_GEM_SHMEM_DRIVER_OPS, + .name = DRIVER_NAME, + .desc = DRIVER_DESC, + .date = DRIVER_DATE, + .major = DRIVER_MAJOR, + .minor = DRIVER_MINOR, + .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, + .fops = &simplekms_fops, +}; + +/* + * Platform driver + */ + +static int simplekms_probe(struct platform_device *pdev) +{ + struct simplekms_device *sdev; + struct drm_device *dev; + int ret; + + sdev = simplekms_device_create(&simplekms_driver, pdev); + if (IS_ERR(sdev)) + return PTR_ERR(sdev); + dev = &sdev->dev; + + ret = drm_dev_register(dev, 0); + if (ret) + return ret; + + return 0; +} + +static int simplekms_remove(struct platform_device *pdev) +{ + struct simplekms_device *sdev = platform_get_drvdata(pdev); + + simplekms_device_cleanup(sdev);If you add the ->disable hook then a comment here that we don't want to shut down to allow fastboot would be nice.+ + return 0; +} + +static struct platform_driver simplekms_platform_driver = { + .driver = { + .name = "simple-framebuffer", /* connect to sysfb */ + }, + .probe = simplekms_probe, + .remove = simplekms_remove, +}; + +module_platform_driver(simplekms_platform_driver); + +MODULE_DESCRIPTION(DRIVER_DESC); +MODULE_LICENSE("GPL v2"); -- 2.27.0Cheers, Daniel
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
