[RFC PATCH v2 1/2] drm/bridge: ti-sn65dsi86: fetch bpc using drm_atomic_state

[Dmitry Baryshkov]

On 11/07/2022 12:10, Sam Ravnborg wrote:
> Hi Dmitry,
> 
> On Mon, Jul 11, 2022 at 10:37:32AM +0300, Dmitry Baryshkov wrote:
>> Rather than reading the pdata->connector directly, fetch the connector
>> using drm_atomic_state. This allows us to make pdata->connector optional
>> (and thus supporting DRM_BRIDGE_ATTACH_NO_CONNECTOR).
>>
>> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>   drivers/gpu/drm/bridge/ti-sn65dsi86.c | 22 ++++++++++++++++------
>>   1 file changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> index d6dd4d99a229..b1b6ed3a8acc 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -779,9 +779,9 @@ static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata)
>>   	regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
>>   }
>>   
>> -static unsigned int ti_sn_bridge_get_bpp(struct ti_sn65dsi86 *pdata)
>> +static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector)
>>   {
>> -	if (pdata->connector->display_info.bpc <= 6)
>> +	if (connector->display_info.bpc <= 6)
>>   		return 18;
>>   	else
>>   		return 24;
>> @@ -796,7 +796,7 @@ static const unsigned int ti_sn_bridge_dp_rate_lut[] = {
>>   	0, 1620, 2160, 2430, 2700, 3240, 4320, 5400
>>   };
>>   
>> -static int ti_sn_bridge_calc_min_dp_rate_idx(struct ti_sn65dsi86 *pdata)
>> +static int ti_sn_bridge_calc_min_dp_rate_idx(struct ti_sn65dsi86 *pdata, unsigned int bpp)
>>   {
>>   	unsigned int bit_rate_khz, dp_rate_mhz;
>>   	unsigned int i;
>> @@ -804,7 +804,7 @@ static int ti_sn_bridge_calc_min_dp_rate_idx(struct ti_sn65dsi86 *pdata)
>>   		&pdata->bridge.encoder->crtc->state->adjusted_mode;
>>   
>>   	/* Calculate minimum bit rate based on our pixel clock. */
>> -	bit_rate_khz = mode->clock * ti_sn_bridge_get_bpp(pdata);
>> +	bit_rate_khz = mode->clock * bpp;
>>   
>>   	/* Calculate minimum DP data rate, taking 80% as per DP spec */
>>   	dp_rate_mhz = DIV_ROUND_UP(bit_rate_khz * DP_CLK_FUDGE_NUM,
>> @@ -1016,12 +1016,21 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
>>   				       struct drm_bridge_state *old_bridge_state)
>>   {
>>   	struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
>> +	struct drm_connector *connector;
>>   	const char *last_err_str = "No supported DP rate";
>>   	unsigned int valid_rates;
>>   	int dp_rate_idx;
>>   	unsigned int val;
>>   	int ret = -EINVAL;
>>   	int max_dp_lanes;
>> +	unsigned int bpp;
>> +
>> +	connector = drm_atomic_get_new_connector_for_encoder(old_bridge_state->base.state,
>> +							     bridge->encoder);
>> +	if (!connector) {
>> +		DRM_DEV_ERROR_RATELIMITED(pdata->dev, "Could not get the connector\n");
> 
>  From the documentation of DRM_DEV_ERROR_RATELIMITED:
>   * NOTE: this is deprecated in favor of drm_err_ratelimited() or
>   * dev_err_ratelimited().
> 
> Can you fix this, so we do not introduce deprecated functions/macros.

Ack. I hesitated between the DRM_DEV and dev_ functions, but decided to 
use the family that was used by the rest of the driver. Will change this 
call to the dev_err in v3.

> 
> 	Sam
> 
> 
>> +		return;
>> +	}
>>   
>>   	max_dp_lanes = ti_sn_get_max_lanes(pdata);
>>   	pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
>> @@ -1047,8 +1056,9 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
>>   	drm_dp_dpcd_writeb(&pdata->aux, DP_EDP_CONFIGURATION_SET,
>>   			   DP_ALTERNATE_SCRAMBLER_RESET_ENABLE);
>>   
>> +	bpp = ti_sn_bridge_get_bpp(connector);
>>   	/* Set the DP output format (18 bpp or 24 bpp) */
>> -	val = (ti_sn_bridge_get_bpp(pdata) == 18) ? BPP_18_RGB : 0;
>> +	val = bpp == 18 ? BPP_18_RGB : 0;
>>   	regmap_update_bits(pdata->regmap, SN_DATA_FORMAT_REG, BPP_18_RGB, val);
>>   
>>   	/* DP lane config */
>> @@ -1059,7 +1069,7 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
>>   	valid_rates = ti_sn_bridge_read_valid_rates(pdata);
>>   
>>   	/* Train until we run out of rates */
>> -	for (dp_rate_idx = ti_sn_bridge_calc_min_dp_rate_idx(pdata);
>> +	for (dp_rate_idx = ti_sn_bridge_calc_min_dp_rate_idx(pdata, bpp);
>>   	     dp_rate_idx < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut);
>>   	     dp_rate_idx++) {
>>   		if (!(valid_rates & BIT(dp_rate_idx)))
>> -- 
>> 2.35.1


-- 
With best wishes
Dmitry