On 10/07/2014 05:56 AM, Mark yao wrote: > On 2014年09月30日 21:31, Andrzej Hajda wrote: >> Hi Mark, > Hi Andrzej, > Sorry for replying late, I have a vacation before. > Thanks for your review. >> On 09/30/2014 03:03 PM, Mark Yao wrote: >>> From: Mark yao <mark.yao@xxxxxxxxxxxxxx> >>> (...) >>> +#ifdef CONFIG_PM_SLEEP >>> +static int rockchip_drm_suspend(struct drm_device *dev, pm_message_t state) >>> +{ >>> + struct drm_connector *connector; >>> + >>> + drm_modeset_lock_all(dev); >>> + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { >>> + int old_dpms = connector->dpms; >>> + >>> + if (connector->funcs->dpms) >>> + connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); >>> + >>> + /* Set the old mode back to the connector for resume */ >>> + connector->dpms = old_dpms; >>> + } >>> + drm_modeset_unlock_all(dev); >>> + >>> + return 0; >>> +} >>> + >>> +static int rockchip_drm_resume(struct drm_device *dev) >>> +{ >>> + struct drm_connector *connector; >>> + >>> + drm_modeset_lock_all(dev); >>> + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { >>> + if (connector->funcs->dpms) >>> + connector->funcs->dpms(connector, connector->dpms); >>> + } >>> + drm_modeset_unlock_all(dev); >>> + >>> + drm_helper_resume_force_mode(dev); >>> + >>> + return 0; >>> +} >>> + >>> +static int rockchip_drm_sys_suspend(struct device *dev) >>> +{ >>> + struct drm_device *drm_dev = dev_get_drvdata(dev); >>> + pm_message_t message; >>> + >>> + if (pm_runtime_suspended(dev)) >>> + return 0; >>> + >>> + message.event = PM_EVENT_SUSPEND; >>> + >>> + return rockchip_drm_suspend(drm_dev, message); >> drm_dev can be NULL here, it can happen when system is suspended >> before all components are bound. It can also contain invalid pointer >> if after successfull drm initialization de-initialization happens for >> some reason. >> >> Some workaround is to check for null here and set drvdata to null on >> master unbind. But I guess it should be protected somehow to avoid races >> in accessing drvdata. > So, can I use the way that check for null here and set drvdata to null > on master unbind? > I don't know which way is better to protect somehow. It seems to be a core problem, I have proposed some solution using drm driver PM callbacks [1] but it appears these callbacks are obsolete, so different solution should be found. According to Russel probably some extension of component framework. As a temporary solution I guess null checks should work in most cases. Regards Andrzej [1]: https://lkml.org/lkml/2014/10/3/60 > > -Mark. >>> +} >>> + >>> +static int rockchip_drm_sys_resume(struct device *dev) >>> +{ >>> + struct drm_device *drm_dev = dev_get_drvdata(dev); >>> + >>> + if (!pm_runtime_suspended(dev)) >>> + return 0; >>> + >>> + return rockchip_drm_resume(drm_dev); >> Ditto. >> >> Regards >> Andrzej >> >> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html