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 <[email protected]>
>>>
(...)
>>> +#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
>>
>>
I've just been discussing how this relates to rk29xx on #etnaviv. I
looked through the patch and it's good to see it's not limited to
supporting Mali GPUs. I see no reason this wouldn't be compatible
with etna_viv for rk29xx (or future Rockchip designs with alternative
GPUs) as long as the rest of the infrastructure to support rk29 is in
place iommu (ARM SMMU?) driver, for example.
IMHO it's vital to keep a logical separation between the video display
hardware and the graphics processor on SoCs.