On 04/04/18 17:23, Laurent Pinchart wrote:
>>>> + WARN(out_width > dispc.feat->ovl_width_max,
>>>> + "Requested OVL width (%d) is larger than can be supported
>>>> (%d).\n",
>>>> + out_width, dispc.feat->ovl_width_max);
>>>
>>> Why don't you return an error here? I don't see a need for WARN here.
>>
>> So here you mean replace the WARN with something like this:
>>
>> if (out_width > dispc.feat->ovl_width_max) {
>> DSSERR("Requested OVL width (%d) is larger than can be supported (%d).
> \n",
>> out_width, dispc.feat->ovl_width_max);
>> return -EINVAL;
>> }
>
> Can this happen ? If we reject invalid settings in omapdrm we should never get
> them here.
That's true. And we should check them in the plane atomic check (but do
we?).
In that case I don't mind a warn there, but you should still return an
error if it happens, instead of continuing with bad config.
>>>> + /* Check if the advertised width exceed what the pipeline can do */
>>>> + if (!r) {
>>>> + struct omap_drm_private *priv = dev->dev_private;
>>>> + u16 width, height;
>>>> +
>>>> + priv->dispc_ops->ovl_get_max_size(&width, &height);
>>>> + if (mode->hdisplay > width)
>>>> + r = -EINVAL;
>>>
>>> You should check the height also.
>>
>> Yeah, I'll fix that.
>
> Unless I'm mistaken the restriction doesn't come from the output side of the
> display controller but from the overlays (planes), right ? Shouldn't it then
> be implemented in the drm_plane_helper_funcs.atomic_check operation ?
Yes, but I don't so. If our planes can support up to, say, 1000. Then we
plug in a monitor with native width of 1100, which omapdrm would accept
happily and try to use it by default. But we can't show fbdev or any
normal setup there, because the planes won't support it. How would we
manage that?
While not strictly correct, I think it's fine to reject videomodes which
can't be shown with a normal full-screen plane.
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
