On 30/05/2023 00:11, Marijn Suijten wrote:
On 2023-05-22 04:16:20, Dmitry Baryshkov wrote:
<snip>
+ if (ctx->dsi->dsc) {
dsi->dsc is always set, thus this condition can be dropped.
I want to leave room for possibly running the panel without DSC (at a
lower resolution/refresh rate, or at higher power consumption if there
is enough BW) by not assigning the pointer, if we get access to panel
documentation: probably one of the magic commands sent in this driver
controls it but we don't know which.
+ drm_dsc_pps_payload_pack(&pps, ctx->dsi->dsc);
+
+ ret = mipi_dsi_picture_parameter_set(ctx->dsi, &pps);
+ if (ret < 0) {
+ dev_err(panel->dev, "failed to transmit PPS: %d\n",
ret);
+ goto fail;
+ }
+ ret = mipi_dsi_compression_mode(ctx->dsi, true);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable compression mode: %d\n",
ret);
+ goto fail;
+ }
+
+ msleep(28);
+ }
+
+ ctx->prepared = true;
+ return 0;
+
+fail:
+ gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+ regulator_disable(ctx->vddio);
+ return ret;
+}
<snip>
+ /* This panel only supports DSC; unconditionally enable it */
On that note I should perhaps reword this.
+ dsi->dsc = dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
I think double assignments are frowned upon.
Ack.
+ if (!dsc)
+ return -ENOMEM;
+
+ dsc->dsc_version_major = 1;
+ dsc->dsc_version_minor = 1;
+
+ dsc->slice_height = 32;
+ dsc->slice_count = 2;
+ // TODO: Get hdisplay from the mode
Would you like to fix the TODO?
I can't unless either migrating to drm_bridge (is that doable?) or
expand drm_panel. That's a larger task, but I don't think this driver
is the right place to track that desire. Should I drop the comment
entirely or reword it?
I'd say, reword it, so that it becomes more obvious why this TODO can
not be fixed at this moment.
+ WARN_ON(1440 % dsc->slice_count);
+ dsc->slice_width = 1440 / dsc->slice_count;
<snip>
- Marijn
--
With best wishes
Dmitry
