On 9/19/22 15:03, Thomas Zimmermann wrote:
> The sku_pixel_limit is a per-device property, similar to the amount
> of available video memory. Move the respective mode-valid test from
> the connector to the mode-config structure.
>
> Signed-off-by: Thomas Zimmermann <[email protected]>
> ---
[...]
> +static enum drm_mode_status udl_mode_config_mode_valid(struct drm_device
> *dev,
> + const struct
> drm_display_mode *mode)
> +{
> + struct udl_device *udl = to_udl(dev);
> +
> + if (udl->sku_pixel_limit) {
> + if (mode->vdisplay * mode->hdisplay > udl->sku_pixel_limit)
> + return MODE_MEM;
> + }
> +
> + return MODE_OK;
> +}
> +
> static const struct drm_mode_config_funcs udl_mode_funcs = {
> .fb_create = drm_gem_fb_create_with_dirty,
> + .mode_valid = udl_mode_config_mode_valid,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = drm_atomic_helper_commit,
> };
It's always confusing to me whether something has to be in the .mode_valid
for drm_mode_config helper function or for the drm_crtc_helper_funcs. This
driver is still using the simple-KMS at this point so that will be in the
udl_simple_display_pipe_mode_valid() if should be the latter.
In this case since it seems to be about a pixel limit, it might make sense
to have this constraint for the DRM mode config. But since it depends on the
{h,v}display, I thought that needed to ask if instead should be for the CRTC.
Any in case,
Reviewed-by: Javier Martinez Canillas <[email protected]>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
