panel-simple-dsi driver was added in 2022 but looks like a duplicate and code is buggy - removal?

[prahal]

I found what a likely duplicate out-of-tree driver after investigating a warning from the kernel build:

   CC [M]  drivers/gpu/drm/panel/panel-simple-dsi.o                                                                                                                                                               
  drivers/gpu/drm/panel/panel-simple-dsi.c: In function 'panel_simple_get_modes':                                                                                                                                  
  drivers/gpu/drm/panel/panel-simple-dsi.c:310:73: warning: passing argument 3 of 'of_get_drm_display_mode' makes pointer from integer without a cast [-Wint-conversion]                                           
    310 |         ret = of_get_drm_display_mode(panel->dev->of_node, mode, p->desc->bus_format,                                                                                                                    
        |                                                                  ~~~~~~~^~~~~~~~of_get_drm_display_mode~~~~                                                                                              
        |                                                                         |                                                                                                                                
        |                                                                         u32 {aka unsigned int}                                                                                                           
  In file included from ./include/drm/drm_crtc.h:32,                                                                                                                                                               
                   from drivers/gpu/drm/panel/panel-simple-dsi.c:13:                                                                                                                                               
  ./include/drm/drm_modes.h:509:66: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'u32' {aka 'unsigned int'}                                                                               
    509 |                             struct drm_display_mode *dmode, u32 *bus_flags,                                                                                                                              
        |                                                             ~~~~~^~~~~~~~~                                                                                                                               
                                                             

This code cannot work as is and a fix would be to add the missing ampersand before the p->desc->bus_format. Still other warning surface when this is fixed.

This driver calls of_get_drm_display_mode against bus_format while panel-simple does against bus_flags (it also has bus_format but it passes it as an argument to  drm_display_info_set_bus_formats).

So it might well be that the driver would pass the wrong arguments.

It has no documentation for its DTS parameters so it is hard to tell if bus_format should be set as bus_flags from the panel-simple documentation or if its user will end up sending bus_format as a bus_flags.

 

There is already a dsi driver in upstream drivers/gpu/drm/panel/panel-simple.c (panel-dsi-simple while this one is panel-simple-dsi).

So I wonder if there is a use in shipping two drivers for the same purpose.

And there are no dts users of this out-of-tree driver panel-simple-dsi.

 

The discussion about its introduction is in: https://github.com/armbian/build/pull/3140

It is told in the commit log to be a port of a rockchip kernel 4.4 driver, but panel-simple has dsi probe since v3.14. I am puzzled rockchip forked panel-simple instead of expanding it.

Rockchip.DRM.Panel.Porting.Guide.pdf has no mention of the bus_format dts parameter so maybe this code path was never used and the buggy code affects no users.

(a few dts parameters in the rockchip code are also not documented in this PDF).

 

@iamdrq you are the author of this driver. Was there code missing in the dis support of panel-simple.c for dsi support, ie is panel-simple-dsi necessary?

Was the aim to have a version of the panel dsi code that is bit-to-bit compatible with the rockchip kernel, so no porting was necessary?

If features were missing could we bake them into the panel-simple.c driver (so they could be tested and one day be upstreamed)?

 

 

[iamdrq]

I'm sorry about this driver has  4 warnings when I checked it,and the  bus_format  actually is bus_flags, I used the wrong name, although it not effect  use, I will fix it when I have time in the future.

This dirver is differ in upstream panel-simple-dsi, panel-simple-dsi actually contain a lot sets panel model.but this panel-dsi-simple is universal driver,you just put init code,config to dts and not need adjsut driver c code when drive a new panel model.

[prahal]

@iamdrqthanks for the feedback.

So it is a "panel dsi code that is bit-to-bit compatible with the rockchip kernel" which is not aimed at ever being upstreamed?

But isn't it opening the door to diverging from upstream definitely?

I mean if the board definitions start to rely on such drivers that are not even proposed upstream because it is against their approved design, these boards will be harder and harder to upstream.

I do not know Armbian policy. I am just used to Debian whose policy is to only accept patches to adapt programs to the distribution and patches that could eventually end up upstreamed.

 

I am interested in understanding the policy of Armbian regarding such eternal divergences.

 

If such patches are deemed good, then after replacing p->desc->bus_format by &p->desc->bus_format (well really bus_flags , though then the OF  bus-format property should be renamed too to avoid confusion), a warning triggers:

 drivers/gpu/drm/panel/panel-simple-dsi.c: In function 'panel_simple_get_modes':                                                                                                                                   
 drivers/gpu/drm/panel/panel-simple-dsi.c:310:66: warning: passing argument 3 of 'of_get_drm_display_mode' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]                            
   310 |         ret = of_get_drm_display_mode(panel->dev->of_node, mode, &p->desc->bus_format,                                                                                                                    
       |                                                                  ^~~~~~~~~~~~~~~~~~~~                                                                                                                     
 In file included from ./include/drm/drm_crtc.h:32,                                                                                                                                                                
                  from drivers/gpu/drm/panel/panel-simple-dsi.c:13:                                                                                                                                                
 ./include/drm/drm_modes.h:509:66: note: expected 'u32 *' {aka 'unsigned int *'} but argument is of type 'const u32 *' {aka 'const unsigned int *'}
   509 |                             struct drm_display_mode *dmode, u32 *bus_flags,                     
       |                                                             ~~~~~^~~~~~~~~  

 

Edited July 18, 2023 by prahal

[iamdrq]

Full remove warning :

ret = of_get_drm_display_mode(panel->dev->of_node, mode, p->desc->bus_format,
change to:
ret = of_get_drm_display_mode(panel->dev->of_node, mode, (u32 *)&p->desc->bus_format,

and delete unused variable line 379,470,525

I am not familiar upstream policy and it aim is not bit-to-bit compatible with the rockchip kernel.

This patch driver make drive a dsi panel easy, so I shared this to armbian build system.I hope it help someone drive dsi panel easy.