On 22 March 2017 at 15:06, Sean Paul <seanpaul@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
