Re: [igt-dev] [[RFC] i-g-t 1/1] lib/igt_kms: Add support for display with non-contiguous pipes

Petri Latvala
2020-06-16T13:43:35Z

On Thu, Jun 11, 2020 at 01:07:24AM +0530, Mohammed Khajapasha wrote:
> Add support for non-contiguous pipe display by allocating
> upper bound pipes array for display. Set the pipe enum name
> to igt pipe for enabled pipes in drm.
>
> Signed-off-by: Mohammed Khajapasha <mohammed.khajapasha@intel.com>
> ---
> lib/igt_kms.c | 30 +++++++++++++++++++++++++-----
> lib/igt_kms.h | 13 ++++++++-----
> 2 files changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index afef5939..2b1cf878 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1921,10 +1921,26 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> * We cache the number of pipes, that number is a physical limit of the
> * hardware and cannot change of time (for now, at least).
> */
> - display->n_pipes = resources->count_crtcs;
> - display->pipes = calloc(sizeof(igt_pipe_t), display->n_pipes);
> + display->n_pipes = IGT_MAX_PIPES;
> + display->pipes = calloc(sizeof(igt_pipe_t), IGT_MAX_PIPES);
> igt_assert_f(display->pipes, "Failed to allocate memory for %d pipes\n", display->n_pipes);
>
> + for(i = 0; i < resources->count_crtcs; i++) {
> + igt_pipe_t *pipe;
> + struct drm_i915_get_pipe_from_crtc_id get_pipe;
> +
> + /* Get right pipe enum from kernel for a pipe */
> + get_pipe.pipe = 0;
> + get_pipe.crtc_id = resources->crtcs[i];
> + do_ioctl(display->drm_fd,
> + DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
> +
> + pipe = &display->pipes[get_pipe.pipe];
> + pipe->pipe = get_pipe.pipe;
> + pipe->enabled = true;
> + pipe->crtc_id = resources->crtcs[i];
> + }
> +

You need to eventually make sure to only do this dancing for i915
devices. Might as well get that done early. For non-i915 devices you
need to make pipe->pipe equal to i directly, and so on.

You also need to store i in the pipe objects. possible_crtcs from a
drm plane refers to the value of i, not the pipe enum. That also needs
to be handled correctly futher down.

> drmSetClientCap(drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
> display->is_atomic = 1;
> @@ -1959,9 +1975,7 @@ void igt_display_require(igt_display_t *display, int drm_fd)
> int j, type;
> uint8_t last_plane = 0, n_planes = 0;
>
> - pipe->crtc_id = resources->crtcs[i];
> pipe->display = display;
> - pipe->pipe = i;
> pipe->plane_cursor = -1;
> pipe->plane_primary = -1;
> pipe->planes = NULL;
> @@ -2409,12 +2423,18 @@ static bool output_is_internal_panel(igt_output_t *output)
>
> igt_output_t **__igt_pipe_populate_outputs(igt_display_t *display, igt_output_t **chosen_outputs)
> {
> - unsigned full_pipe_mask = (1 << (display->n_pipes)) - 1, assigned_pipes = 0;
> + unsigned full_pipe_mask, assigned_pipes = 0;
> igt_output_t *output;
> int i, j;
>
> memset(chosen_outputs, 0, sizeof(*chosen_outputs) * display->n_pipes);
>
> + for( i = 0; i < display->n_pipes; i++) {
> + igt_pipe_t *pipe = &display->pipes[i];
> + if (pipe->enabled)
> + full_pipe_mask |= (1 << pipe->pipe);

full_pipe_mask operates on config->valid_crtc_idx_mask later in this
function, so it's one of those that needs to operate on the index (i),
not the pipe->pipe value.

--
Petri Latvala