Re: [PATCH] drm/msm/dsi: add protection against NULL dsi device

[Jeffrey Hugo <jhugo@codeaurora.org>]

On 3/7/2019 3:02 PM, Abhinav Kumar wrote:
> On 2019-03-07 13:59, Sean Paul wrote:
>> On Wed, Mar 06, 2019 at 05:28:54PM -0800, Abhinav Kumar wrote:
>>> When panel probe happens after DSI probe, the DSI probe
>>> is deferred as per current design. In the probe defer path
>>> dsi device is destroyed. This NULL dsi device could be
>>> deferenced by the panel probe in the mipi_dsi_attach path.
>>>
>>> Check for NULL dsi device before accessing it.
>>
>> It would be really nice to sort all of this out in a manner that's not
>> sprinkling NULL checks around the driver. I spent 5 minutes looking 
>> around and
>> couldn't really make sense of how all of these pieces interact, but 
>> this seems
>> like it might be an architectural problem (perhaps since dpu was using 
>> its own
>> panel stuff instead of drm_panel?).
>>
>> Anyways, it'd be nice to fix that.
>>
>> In the meantime, could you please add a big comment like the !dev 
>> check in this
>> function explaining how this situation can come to pass? That way the 
>> knowledge
>> isn't lost and whoever comes along to clean up all of these probe 
>> checks will
>> have some clue as to what's going on.
>>
>> Sean
> [Abhinav] Sure Sean, will add a detailed comment to explain the scenario

Abhinav, it looks like this may have dropped off your radar.  Do you 
know when you'll come back to it?

>>
>>>
>>> Reported-by: Jeffrey Hugo <jhugo@codeaurora.org>
>>> Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>
>>> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>>> ---
>>>  drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
>>> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> index 80aa634..cc2569d 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
>>> @@ -769,7 +769,7 @@ bool msm_dsi_manager_cmd_xfer_trigger(int id, u32 
>>> dma_base, u32 len)
>>>  void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags)
>>>  {
>>>      struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
>>> -    struct drm_device *dev = msm_dsi->dev;
>>> +    struct drm_device *dev = msm_dsi ? msm_dsi->dev : NULL;
>>>      struct msm_drm_private *priv;
>>>      struct msm_kms *kms;
>>>      struct drm_encoder *encoder;
>>> -- 
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>>> Forum,
>>> a Linux Foundation Collaborative Project
>>>


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.