Hi Am 29.09.22 um 15:20 schrieb Javier Martinez Canillas:
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 <tzimmermann@xxxxxxx> ---[...]+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.
We're testing modes against limitations of the driver or hardware. The rule of thumb is to use the mode_valid function of the component that imposes the limitation. In the case at hand, the limit is the maximum number of pixels that the adapter can store. So it goes into the device-wide mode_valid.
Best regards Thomas
Any in case, Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
