Re: [PATCH 1/2] drm: Introduce crtc->mode_valid() callback

[Andrzej Hajda <a.hajda@samsung.com>]

Hi Jose,

On 26.04.2017 12:48, Jose Abreu wrote:
> Some crtc's may have restrictions in the mode they can display. In
> this patch a new callback (crtc->mode_valid()) is introduced that
> is called at the same stage of connector->mode_valid() callback.
>
> This shall be implemented if the crtc has some sort of restriction
> so that we don't probe modes that will fail in the commit() stage.
> For example: A given crtc may be responsible to set a clock value.
> If the clock can not produce all the values for the available
> modes then this callback can be used to restrict the number of
> probbed modes to only the ones that can be displayed.
>
> If the crtc does not implement the callback then the behaviour will
> remain the same. Also, for a given set of crtcs that can be bound to
> the connector, if at least one can display the mode then the mode
> will be probbed.

I see few possible issues/improvements here:
1. crtc can have different constraints depending on the encoder it is
connected to, theoretically reverse dependency is also possible, but I
am not aware of such hw.
2. there also could be similar dependency constrains between connector
and encoder, I guess.
3. encoders and bridges should have also possibility to validate modes,
they also have constrains, btw encoder_slave have such callback.

Regarding 1st and 2nd point, maybe it would be good to validate modes
according to topology described in drm_mode_get_connector::encoder_id
and drm_mode_get_encoder::crtc_id:
a) if connector is not connected to any encoder report to userspace
modes filtered only by connector::mode_valid,
b) if connector is connected to encoder, report modes filtered by
connector, encoder and attached bridges,
c) if full pipeline is constructed, report modes filtered by all members
of the pipeline.

Of course with this approach drm_mode_get_connector userspace should be
aware of the fact that it should re-call drm_mode_get_connector after
topology change to update supported modes.

Regards
Andrzej