linux-kernel - Re: [PATCH v1 2/2] drm: panel: Add driver for Himax HX8279 and Startek KD070FHFID078

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a72cb224-2cae-44a6-90d5-87d9f4f80044@wanadoo.fr>
Date: Tue, 11 Feb 2025 20:16:15 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: airlied@...il.com, conor+dt@...nel.org, devicetree@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, kernel@...labora.com, krzk+dt@...nel.org,
 linux-kernel@...r.kernel.org, maarten.lankhorst@...ux.intel.com,
 mripard@...nel.org, neil.armstrong@...aro.org, pablo.sun@...iatek.com,
 quic_jesszhan@...cinc.com, robh@...nel.org, simona@...ll.ch,
 tzimmermann@...e.de
Subject: Re: [PATCH v1 2/2] drm: panel: Add driver for Himax HX8279 and
 Startek KD070FHFID078

Le 11/02/2025 à 12:44, AngeloGioacchino Del Regno a écrit :
> Add a driver for the Himax HX8279-D MIPI-DSI DriverIC with support
> for the Startek KX070FHFID078 7.0" 1200x1920 IPS panel, found on
> various MediaTek Genio Evaluation Kit boards.

...

> +	if (!hx->skip_goa_config) {
> +		if ((desc->goa_stv_lead_time_ck > HX8279_P3_GOA_STV_LEAD) ||
> +		    (desc->goa_ckv_lead_time_ck > HX8279_P3_GOA_CKV_LEAD) ||
> +		    (desc->goa_ckv_dummy_vblank_num > HX8279_P3_GOA_CKV_DUMMY))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid lead timings in GOA config\n");
> +		/*
> +		 * Don't perform zero check for polarity and start position, as
> +		 * both pol=0 and start=0 are valid configuration values.
> +		 */
> +		num_clr = 0;
> +		for (i = 0; i < ARRAY_SIZE(desc->goa_clr_start_pos); i++) {
> +			if (desc->goa_clr_start_pos[i] < 0)
> +				continue;
> +			else if (desc->goa_clr_start_pos[i] > HX8279_P3_GOA_CLR_CFG_STARTPOS)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "Invalid start position for CLR%d\n", i + 1);
> +			else
> +				num_clr++;
> +		}
> +		if (!num_clr)
> +			return -EINVAL;
> +
> +		for (i = 0; i < ARRAY_SIZE(desc->goa_clr_polarity); i++) {
> +			if (num_clr < 0)
> +				return -EINVAL;
> +
> +			if (desc->goa_clr_polarity[i] < 0)
> +				continue;
> +			else if (desc->goa_clr_polarity[i] > 1)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "Invalid polarity for CLR%d\n", i + 1);
> +			else if (desc->goa_clr_polarity[i] >= 0)

The if () looks superfluous.

> +				num_clr--;
> +		}
> +	}
> +
> +	/* MIPI Configuration validation */
> +	if (!desc->bta_tlpx && !desc->lhs_settle_time_by_osc25 &&
> +	    !desc->ths_settle_time && !desc->timing_unk_b8 &&
> +	    !desc->timing_unk_bc && !desc->timing_unk_d6)
> +		hx->skip_mipi_timing = true;
> +
> +	/* Gamma Configuration validation */
> +	if (desc->gamma_ctl > (HX8279_P6_GAMMA_POCGM_CTL | HX8279_P6_GAMMA_POGCMD_CTL))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int hx8279_probe(struct mipi_dsi_device *dsi)
> +{

...

> +	/* If the panel is connected on two DSIs then DSI0 left, DSI1 right */
> +	if (hx->desc->is_dual_dsi) {
> +		const struct mipi_dsi_device_info *info = &hx->desc->dsi_info;
> +		struct mipi_dsi_host *dsi_r_host;
> +		struct device_node *dsi_r;
> +
> +		dsi_r = of_graph_get_remote_node(dsi->dev.of_node, 1, -1);
> +		if (!dsi_r)
> +			return dev_err_probe(dev, -ENODEV, "Cannot get secondary DSI node.\n");
> +
> +		dsi_r_host = of_find_mipi_dsi_host_by_node(dsi_r);
> +		of_node_put(dsi_r);
> +		if (!dsi_r_host)
> +			return dev_err_probe(dev, -EPROBE_DEFER,
> +					     "Cannot get secondary DSI host\n");
> +
> +		hx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi_r_host, info);
> +		if (IS_ERR(hx->dsi[1]))
> +			return dev_err_probe(dev, PTR_ERR(hx->dsi[1]),
> +					     "Cannot get secondary DSI node\n");
> +		num_dsis++;
> +		mipi_dsi_set_drvdata(hx->dsi[1], hx);
> +	}
> +
> +	hx->dsi[0] = dsi;
> +	mipi_dsi_set_drvdata(dsi, hx);
> +
> +	drm_panel_init(&hx->panel, dev, &hx8279_panel_funcs, DRM_MODE_CONNECTOR_DSI);
> +
> +	ret = drm_panel_of_backlight(&hx->panel);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to get backlight\n");
> +
> +	drm_panel_add(&hx->panel);
> +
> +	for (i = 0; i < num_dsis; i++) {
> +		hx->dsi[i]->lanes = hx->desc->num_lanes;
> +		hx->dsi[i]->format = MIPI_DSI_FMT_RGB888;
> +
> +		hx->dsi[i]->mode_flags = MIPI_DSI_CLOCK_NON_CONTINUOUS |
> +					 MIPI_DSI_MODE_LPM;
> +
> +		if (hx->desc->mode_data[0].is_video_mode)
> +			hx->dsi[i]->mode_flags |= MIPI_DSI_MODE_VIDEO |
> +						  MIPI_DSI_MODE_VIDEO_SYNC_PULSE;
> +
> +		ret = devm_mipi_dsi_attach(dev, hx->dsi[i]);
> +		if (ret < 0) {
> +			/* If we fail to attach to either host, we're done */
> +			if (num_dsis == 2)
> +				mipi_dsi_device_unregister(hx->dsi[1]);

Is it needed?
(devm_mipi_dsi_device_register_full() was used)

> +
> +			return dev_err_probe(dev, ret,
> +					     "Cannot attach to DSI%d host.\n", i);
> +		}
> +	}
> +
> +	/* Make sure we start in a known state: panel off */
> +	gpiod_set_value_cansleep(hx->reset_gpio, 0);
> +	gpiod_set_value_cansleep(hx->enable_gpio, 0);
> +
> +	return 0;
> +}

...

CJ