On 22 March 2017 at 15:06, Sean Paul seanpaul@chromium.org wrote:
On Wed, Mar 22, 2017 at 02:36:27PM +0000, Emil Velikov wrote:
Hi Sean,
On 16 March 2017 at 22:08, Sean Paul seanpaul@chromium.org wrote:
This series pulls out the power-sequencing code from panel-simple into a panel-common helper library. This allows drivers that cannot leverage panel-simple to share some code.
I've converted the 2 sharp mipi drivers, and Chris Zhong's driver on the list can also be converted. I haven't checked any other drivers, but I suspect we'll see the same code blocks there too.
I'm sure there's more we can pull out of the various drivers, but this seems like a good place to start talking about how to share common panel code across drivers.
Fwiw I think that the idea is good, but I'm wondering on the following architectural questions:
Hey Emil, Thanks for your feedback!
- Shouldn't prepared and enabled be part of struct drm_panel ?
I don't think so. Not all panels need to worry about keeping track of the prepared/enabled state.
You're correct - I did not notice those before. Yet, seems like adding the trivial prepared/enabled check will be beneficial to them ?
Unrelated tidbits/ideas, while skimming through: - lg-lg4573 struct lg4573::vm - never set/used. Might want a prepare/unprepare hook to manage power on/off - panasonic-vvx10f034n00 struct wuxga_nt_panel::mode - set but unused - sharp-ls043t1le01 struct sharp_nt_panel::mode - set but unused
- Would it be better to subclass struct panel_common around struct drm_panel ?
I might be threading the thin line of "midlayer vs helpers" here, so please let me know if I've got it wrong.
Yeah, you could do either. I was going for something more akin to the helpers we already have. If you went the subclass route, the drivers would need to subclass panel_common, which would subclass drm_panel. I figured it was too much unraveling to get at the important bits in the hooks that provide a drm_panel pointer. Nothing that a few to_* helpers couldn't solve, though.
I was also thinking we could subclass panel_common if there's an opportunity to get something like mipi_dcs_panel_common to reduce the copypasta amongst mipi panels which execute a common dcs recipe.
Indeed - wrapping panel_common around drm_panel might not be that good of idea.
Thanks Emil