@@ -162,8 +141,8 @@ static int nt36672a_panel_prepare(struct drm_panel
*panel)
dsi_ctx.accum_err = nt36672a_panel_power_on(pinfo);
/* send first part of init cmds */
- nt36672a_send_cmds(&dsi_ctx, pinfo->desc->on_cmds_1,
- pinfo->desc->num_on_cmds_1);
+ if (pinfo->desc->send_init_cmds_1)
+ pinfo->desc->send_init_cmds_1(&dsi_ctx);
mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
@@ -173,8 +152,8 @@ static int nt36672a_panel_prepare(struct drm_panel
*panel)
mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
/* Send rest of the init cmds */
- nt36672a_send_cmds(&dsi_ctx, pinfo->desc->on_cmds_2,
- pinfo->desc->num_on_cmds_2);
+ if (pinfo->desc->send_init_cmds_2)
+ pinfo->desc->send_init_cmds_2(&dsi_ctx);
mipi_dsi_msleep(&dsi_ctx, 120);
Hi!
Here, the split of panel init sequence into 2 separate functions _1 / _2
is completely artificial and completely unnecessary. For some unknown
reason the initial version of driver had initialization procedure cut in
2 parts on the boundary of exit_sleep_mode + set_display_on commands.
I think the whole init sequence should be glued together into one big
send_init_cmds() callback and have all these calls:
mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
/* 0x46 = 70 ms delay */
mipi_dsi_msleep(&dsi_ctx, 70);
mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
included in the middle. This opens up a possibility for other panels
based on nt36672a to be supported in this driver, which don't have the
init sequence split this way. The location of exit_sleep_mode and
set_display_on commands there is different. And, importantly, the delay
is different too.
I once did a very similar change to this driver [1] to add support for
more panels (ignore touchscreen changes there, look only at panel).
What is also important, that change was also tested on Xiaomi Poco F1
(xiaomi-beryllium) phone, which is the main user of this panel.
Unfortunately I never got to send it, even though I wanted to..
What do you think?
[1] https://gitlab.com/sdm845-mainline/linux/-/merge_requests/131
Hi Alexey,
Thanks, that makes sense and I agree the fully panel-owned init sequence
would likely be a cleaner abstraction long term, especially for
supporting additional NT36672A panel variants with different sequencing
requirements.
For this patch, though, I was trying to keep the scope limited to the
original refactor requested during earlier review:
remove the command table abstraction,
inline the command sequences,
and preserve the existing init flow/behavior as closely as possible.
Since moving exit_sleep_mode, delays, and set_display_on into the panel
callback changes the sequencing model itself, I was thinking it may be
better handled as a follow-up cleanup/refactor patch to avoid mixing
behavioral restructuring into this series.
If that sounds reasonable, I'd prefer to keep this patch focused and
address the sequencing abstraction separately afterward.
Regards,
Chintan
