Re: [PATCH v4 3/3] drm/tiny: Add MIPI DBI compatible SPI driver

["Noralf Trønnes" <noralf@tronnes.org>]

Den 20.02.2022 11.04, skrev Sam Ravnborg:
> Hi Noralf,
> 
>>> +static int panel_mipi_dbi_get_mode(struct mipi_dbi_dev *dbidev, struct drm_display_mode *mode)
>>> +{
>>> +	struct device *dev = dbidev->drm.dev;
>>> +	u32 width_mm = 0, height_mm = 0;
>>> +	struct display_timing timing;
>>> +	struct videomode vm;
>>> +	int ret;
>>> +
>>> +	ret = of_get_display_timing(dev->of_node, "panel-timing", &timing);
>>> +	if (ret) {
>>> +		dev_err(dev, "%pOF: failed to get panel-timing (error=%d)\n", dev->of_node, ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	videomode_from_timing(&timing, &vm);
>>> +
>>> +	if (!vm.hactive || vm.hfront_porch || vm.hsync_len ||
>>> +	    (vm.hback_porch + vm.hactive) > 0xffff ||
>>> +	    !vm.vactive || vm.vfront_porch || vm.vsync_len ||
>>> +	    (vm.vback_porch + vm.vactive) > 0xffff ||
>>> +	    vm.flags) {
>>> +		dev_err(dev, "%pOF: panel-timing out of bounds\n", dev->of_node);
>>> +		return -EINVAL;
>>> +	}
>> We should have a helper that implements this. Maybe the display_timing
>> => display_mode helper could do it.
> 
> It would be nice with a drm_display_timing_to_mode() but that can come
> later - the comment above should not be understood that I consider it
> mandatory for this driver.
> 

I did consider adding an of_get_drm_panel_mode() fashioned after
of_get_drm_display_mode() but I didn't find any other driver that would
actually be able to use it and I would have to do some substraction to
get back the {h,v}front_porch values that I need and the optional pixel
clock calculation becomes more complex acting from a drm_display_mode so
I decided against it.

Looking at it now, what I could do is add a function like what
of_get_videomode() does for "display-timings":

/**
 * of_get_panel_videomode - get the panel-timing videomode from devicetree
 * @np: devicenode containing the panel-timing subnode
 * @vm: returns the videomode
 *
 * Returns:
 * Zero on success, negative error code on failure.
 **/
int of_get_panel_videomode(struct device_node *np, struct videomode *vm)
{
	struct display_timing timing;
	int ret;

	ret = of_get_display_timing(np, "panel-timing", &timing);
	if (ret)
		return ret;

	videomode_from_timing(&timing, vm);

	return 0;
}

This could also be used by panel-lvds and 2 fbdev drivers, the other
panel-timing users need/use the display_timing itself, some for bounds
checking.

Noralf.