On 8/12/26 7:55 AM, Neil Armstrong wrote:
>> +static int nt37703_get_modes(struct drm_panel *panel, struct drm_connector
>> *connector)
>> +{
>> + struct panel_info *pinfo = to_panel_info(panel);
>> + int i;
>> +
>> + for (i = 0; i < pinfo->desc->num_modes; i++) {
>> + const struct drm_display_mode *m = &pinfo->desc->modes[i];
>> + struct drm_display_mode *mode;
>> +
>> + mode = drm_mode_duplicate(connector->dev, m);
>> + if (!mode) {
>> + dev_err(panel->dev, "Failed to add mode %ux%u@%u\n",
>> m->hdisplay,
>> + m->vdisplay, drm_mode_vrefresh(m));
>> + return -ENOMEM;
>> + }
>> +
>> + mode->type = DRM_MODE_TYPE_DRIVER;
>> + if (i == 0)
>> + mode->type |= DRM_MODE_TYPE_PREFERRED;
>> +
>> + drm_mode_set_name(mode);
>> + drm_mode_probed_add(connector, mode);
>> + }
>> +
>> + connector->display_info.width_mm = pinfo->desc->width_mm;
>> + connector->display_info.height_mm = pinfo->desc->height_mm;
>> +
>> + return pinfo->desc->num_modes;
>
> The only supported panel only supports a single mode, just support
> a single mode for now and drop this open coded support for multiple modes.
>
> You can add it later if somehow you need multiple modes.
The idea is that for each new panel, instead of modifying the driver,
only these structures are added:
- (panel)_modes[]
- (panel)_init_sequence()
- struct drm_dsc_config (panel)_dsc
- (panel)_desc
And modifying the of_device_id table afterwards.
So dropping only the modes would mean getting rid of all these
altogether, otherwise the only supported mode would be hardcoded which
renders these structures useless.
[...]
>> +static int nt37703_probe(struct mipi_dsi_device *dsi)
>> +{
>> + struct device *dev = &dsi->dev;
>> + struct panel_info *pinfo;
>> + int ret;
>> +
>> + pinfo = devm_drm_panel_alloc(dev, struct panel_info, panel,
>> &nt37703_panel_funcs,
>> + DRM_MODE_CONNECTOR_DSI);
>> + if (IS_ERR(pinfo))
>> + return PTR_ERR(pinfo);
>> +
>> + pinfo->desc = of_device_get_match_data(dev);
>> + if (IS_ERR(pinfo->desc))
>> + return PTR_ERR(pinfo->desc);
>> +
>> + ret = devm_regulator_bulk_get_const(dev, ARRAY_SIZE(nt37703_supplies),
>> nt37703_supplies,
>> + &pinfo->supplies);
>> + if (ret < 0)
>> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
>> +
>> + pinfo->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>> + if (IS_ERR(pinfo->reset_gpio))
>> + return dev_err_probe(dev, PTR_ERR(pinfo->reset_gpio),
>> + "Failed to get reset-gpios\n");
>> +
>> + pinfo->dsi = dsi;
>> + mipi_dsi_set_drvdata(dsi, pinfo);
>> +
>> + dsi->lanes = pinfo->desc->lanes;
>> + dsi->format = pinfo->desc->format;
>> + dsi->mode_flags = pinfo->desc->mode_flags;
>> +
>> + pinfo->panel.prepare_prev_first = true;
>> +
>> + pinfo->panel.backlight = nt37703_create_backlight(dsi);
>> + if (IS_ERR(pinfo->panel.backlight))
>> + return dev_err_probe(dev, PTR_ERR(pinfo->panel.backlight),
>> + "Failed to create backlight\n");
>> +
>> + drm_panel_add(&pinfo->panel);
>
> Use devm_ variant
Ack
>> +
>> + /* This panel only supports DSC; unconditionally enable it */
>> + pinfo->dsc = *(pinfo->desc->dsc);
>> + dsi->dsc = &pinfo->dsc;
>> +
>> + ret = mipi_dsi_attach(dsi);
>
> Use devm_ variant
Ack
>> + if (ret < 0) {
>> + drm_panel_remove(&pinfo->panel);
>> + return dev_err_probe(dev, ret, "Failed to attach to DSI
>> host\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void nt37703_remove(struct mipi_dsi_device *dsi)
>> +{
>> + struct panel_info *pinfo = mipi_dsi_get_drvdata(dsi);
>> + int ret;
>> +
>> + ret = mipi_dsi_detach(dsi);
>> + if (ret < 0)
>> + dev_err(&dsi->dev, "Failed to detach from DSI host: %d\n", ret);
>> +
>> + drm_panel_remove(&pinfo->panel);
>> +}
>
> And drop the remove()
Ack
Thanks for reviewing.
Regards,
Esteban
