On 09/05/2022 12:35, Thomas Zimmermann wrote:
Split up the connector's mode_valid helper into a simple-pipe and a mode-config helper. The simple-pipe helper tests for display-size limits while the mode-config helper tests for memory-bandwidth limits.
Also add the mgag200_ prefix to mga_vga_calculate_mode_bandwidth() and comment on the function's purpose.
Signed-off-by: Thomas Zimmermann tzimmermann@suse.de
drivers/gpu/drm/mgag200/mgag200_mode.c | 146 ++++++++++++------------- 1 file changed, 69 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 92d3ae9489f0..a808827d7a2c 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -681,38 +681,27 @@ static int mgag200_vga_connector_helper_get_modes(struct drm_connector *connecto return ret; }
-static uint32_t mga_vga_calculate_mode_bandwidth(struct drm_display_mode *mode,
int bits_per_pixel)-{
- uint32_t total_area, divisor;
- uint64_t active_area, pixels_per_second, bandwidth;
- uint64_t bytes_per_pixel = (bits_per_pixel + 7) / 8;
- divisor = 1024;
- if (!mode->htotal || !mode->vtotal || !mode->clock)
return 0;- active_area = mode->hdisplay * mode->vdisplay;
- total_area = mode->htotal * mode->vtotal;
- pixels_per_second = active_area * mode->clock * 1000;
- do_div(pixels_per_second, total_area);
- bandwidth = pixels_per_second * bytes_per_pixel * 100;
- do_div(bandwidth, divisor);
+static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
- .get_modes = mgag200_vga_connector_helper_get_modes,
+};
- return (uint32_t)(bandwidth);
-} +static const struct drm_connector_funcs mga_vga_connector_funcs = {
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
-#define MODE_BANDWIDTH MODE_BAD +/*
- Simple Display Pipe
- */
-static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)+static enum drm_mode_status +mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)
- struct drm_device *dev = connector->dev;
- struct mga_device *mdev = to_mga_device(dev);
- int bpp = 32;
struct mga_device *mdev = to_mga_device(pipe->crtc.dev);
if (IS_G200_SE(mdev)) { u32 unique_rev_id = mdev->model.g200se.unique_rev_id;
@@ -722,42 +711,17 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector, return MODE_VIRTUAL_X; if (mode->vdisplay > 1200) return MODE_VIRTUAL_Y;
if (mga_vga_calculate_mode_bandwidth(mode, bpp)> (24400 * 1024))return MODE_BANDWIDTH;} else if (unique_rev_id == 0x02) { if (mode->hdisplay > 1920) return MODE_VIRTUAL_X; if (mode->vdisplay > 1200) return MODE_VIRTUAL_Y;
if (mga_vga_calculate_mode_bandwidth(mode, bpp)> (30100 * 1024))return MODE_BANDWIDTH;} else {if (mga_vga_calculate_mode_bandwidth(mode, bpp)> (55000 * 1024))return MODE_BANDWIDTH;} } else if (mdev->type == G200_WB) { if (mode->hdisplay > 1280) return MODE_VIRTUAL_X; if (mode->vdisplay > 1024) return MODE_VIRTUAL_Y;
if (mga_vga_calculate_mode_bandwidth(mode, bpp) >(31877 * 1024))return MODE_BANDWIDTH;} else if (mdev->type == G200_EV &&
(mga_vga_calculate_mode_bandwidth(mode, bpp)> (32700 * 1024))) {return MODE_BANDWIDTH;} else if (mdev->type == G200_EH &&
(mga_vga_calculate_mode_bandwidth(mode, bpp)> (37500 * 1024))) {return MODE_BANDWIDTH;} else if (mdev->type == G200_ER &&
(mga_vga_calculate_mode_bandwidth(mode,bpp) > (55000 * 1024))) {return MODE_BANDWIDTH;}
if ((mode->hdisplay % 8) != 0 || (mode->hsync_start % 8) != 0 ||
@@ -775,30 +739,6 @@ static enum drm_mode_status mga_vga_mode_valid(struct drm_connector *connector, return MODE_OK; }
-static const struct drm_connector_helper_funcs mga_vga_connector_helper_funcs = {
- .get_modes = mgag200_vga_connector_helper_get_modes,
- .mode_valid = mga_vga_mode_valid,
-};
-static const struct drm_connector_funcs mga_vga_connector_funcs = {
- .reset = drm_atomic_helper_connector_reset,
- .fill_modes = drm_helper_probe_single_connector_modes,
- .destroy = drm_connector_cleanup,
- .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
- .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-};
-/*
- Simple Display Pipe
- */
-static enum drm_mode_status -mgag200_simple_display_pipe_mode_valid(struct drm_simple_display_pipe *pipe,
const struct drm_display_mode *mode)-{
- return MODE_OK;
-}
- static void mgag200_handle_damage(struct mga_device *mdev, struct drm_framebuffer *fb, struct drm_rect *clip, const struct iosys_map *map)
@@ -1009,6 +949,31 @@ static const uint64_t mgag200_simple_display_pipe_fmtmods[] = {
- Mode config
*/
+/* Calculates a mode's required memory bandwidth (in KiB/sec). */ +static uint32_t mgag200_calculate_mode_bandwidth(const struct drm_display_mode *mode,
unsigned int bits_per_pixel)+{
- uint32_t total_area, divisor;
- uint64_t active_area, pixels_per_second, bandwidth;
- uint64_t bytes_per_pixel = (bits_per_pixel + 7) / 8;
- divisor = 1024;
- if (!mode->htotal || !mode->vtotal || !mode->clock)
return 0;- active_area = mode->hdisplay * mode->vdisplay;
- total_area = mode->htotal * mode->vtotal;
- pixels_per_second = active_area * mode->clock * 1000;
- do_div(pixels_per_second, total_area);
- bandwidth = pixels_per_second * bytes_per_pixel * 100;
- do_div(bandwidth, divisor);
- return (uint32_t)bandwidth;
+}
- static enum drm_mode_status mgag200_mode_config_mode_valid(struct drm_device *dev, const struct drm_display_mode *mode) {
@@ -1024,6 +989,33 @@ static enum drm_mode_status mgag200_mode_config_mode_valid(struct drm_device *de if (fbpages > max_fbpages) return MODE_MEM;
- if (IS_G200_SE(mdev)) {
u32 unique_rev_id = mdev->model.g200se.unique_rev_id;if (unique_rev_id == 0x01) {if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (24400 * 1024))return MODE_BAD;} else if (unique_rev_id == 0x02) {if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (30100 * 1024))return MODE_BAD;} else {if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (55000 * 1024))return MODE_BAD;}- } else if (mdev->type == G200_WB) {
if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (31877 * 1024))return MODE_BAD;- } else if (mdev->type == G200_EV) {
if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (32700 * 1024))return MODE_BAD;- } else if (mdev->type == G200_EH) {
if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (37500 * 1024))return MODE_BAD;- } else if (mdev->type == G200_ER) {
if (mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > (55000 * 1024))return MODE_BAD;- }
- return MODE_OK; }
One suggestion to avoid too much repetition:
static int mgag200_get_bandwidth_kbps(mdev) {
if (IS_G200_SE(mdev)) { u32 unique_rev_id = mdev->model.g200se.unique_rev_id;
if (unique_rev_id == 0x01) { return 24400; } else if (unique_rev_id == 0x02) { return 30100; ...
} else if (mdev->type == G200_ER) { return 55000; } /* No bandwidth defined */ return 0; }
then in mgag200_mode_config_mode_valid()
int g200_bandwidth = mgag200_get_bandwidth_kbps(mdev);
if (g200_bandwidth && mgag200_calculate_mode_bandwidth(mode, max_bpp * 8) > g200_bandwidth * 1024) return MODE_BAD;
I've also tested this patchset, and have seen no regression.
you can add
Reviewed-by: Jocelyn Falempe jfalempe@redhat.com Tested-by: Jocelyn Falempe jfalempe@redhat.com
for the whole series.