[PATCH] drm/tilcdc: Force recalculation of vrefresh in tilcdc_crtc_set_mode()

[Jyri Sarha]


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 10/12/17 12:53, Kevin Hao wrote:
> On Thu, Oct 12, 2017 at 12:39:25PM +0300, Jyri Sarha wrote:
>> We are using the vrefresh to check if we are too close to vertical
>> sync to update the two framebuffer DMA registers and risk a
>> collision. The vrefresh is coming from user space and normally it is
>> not used for anything. For instance xserver leaves vrefresh to zero
>> causing a division by zero when setting the mode. We want to make sure
>> the value is valid and force its recalculation in
>> tilcdc_crtc_set_mode().
>>
>> Reported-by: Kevin Hao <kexin.hao at windriver.com>
>> Fixes: 11abbc9f39e0 ("drm/tilcdc: Set framebuffer DMA address to HW only if CRTC is enabled")
>> Cc: <stable at vger.kernel.org> # v4.11+
>> Signed-off-by: Jyri Sarha <jsarha at ti.com>
>> ---
>>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> index 704db24..aa5e87c 100644
>> --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
>> @@ -459,6 +459,16 @@ static void tilcdc_crtc_set_mode(struct drm_crtc *crtc)
>>  	drm_framebuffer_get(fb);
>>  
>>  	crtc->hwmode = crtc->state->adjusted_mode;
>> +
>> +	/* We are using the vrefresh to check if we are too close to
>> +	 * vertical sync to update the two framebuffer DMA registers
>> +	 * and risk a collision. The vrefresh is coming from user
>> +	 * space and normally it is not used for anything. We want to
>> +	 * make sure the value is valid and force its recalculation
>> +	 * here.
>> +	 */
>> +	crtc->hwmode.vrefresh = 0;
> 
> Why do we need this?
> 

The drm_mode_vrefresh() [1] checks if the vrefresh is > 0 and only
calculates it if it is not. I want to make sure the value is correct by
recalculating it always. This is to prevent buggy user space SW from
causing undefined behaviour.

Best regards,
Jyri

[1] int drm_mode_vrefresh(const struct drm_display_mode *mode)
{
	int refresh = 0;
	unsigned int calc_val;

	if (mode->vrefresh > 0)
		refresh = mode->vrefresh;
	else if (mode->htotal > 0 && mode->vtotal > 0) {
		int vtotal;
		vtotal = mode->vtotal;
		/* work out vrefresh the value will be x1000 */
		calc_val = (mode->clock * 1000);
		calc_val /= mode->htotal;
		refresh = (calc_val + vtotal / 2) / vtotal;

		if (mode->flags & DRM_MODE_FLAG_INTERLACE)
			refresh *= 2;
		if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
			refresh /= 2;
		if (mode->vscan > 1)
			refresh /= mode->vscan;
	}
	return refresh;
}