Re: [PATCH i-g-t v3] tests/kms_content_protection: Optimize framebuffer management

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

[snip]

> > @@ -869,6 +893,7 @@ test_content_protection_mst(int content_type)
> >   	igt_crtc_t *crtc;
> >   	bool pipe_found;
> >   	igt_output_t *hdcp_mst_output[IGT_MAX_PIPES];
> > +	drmModeModeInfo *mode;
> >   
> >   	for_each_crtc(display, crtc)
> >   		max_pipe++;
> > @@ -889,7 +914,34 @@ test_content_protection_mst(int content_type)
> >   		igt_assert_f(pipe_found, "No valid pipe found for
> > %s\n", output->name);
> >   
> >   		igt_output_set_crtc(output, crtc);
> > -		prepare_modeset_on_mst_output(output, false);
> > +
> > +		mode = igt_output_get_mode(output);
> > +
> > +		/*
> > +		 * Check if framebuffers already exist for this
> > crtc from create_fbs().
> > +		 * Since non-MST tests run first and share the
> > same data.fbs[] array,
> > +		 * the existing FB may have a different resolution
> > than this MST output
> > +		 * requires. If so, properly detach planes, remove
> > old FBs, and recreate
> > +		 * with the correct dimensions for this MST
> > output.
> > +		 */
> > +		if (data.fbs[crtc->crtc_index].red.fb_id) {
> > +			igt_plane_t *primary =
> > igt_output_get_plane_type(output,
> > +								
> > 	 DRM_PLANE_TYPE_PRIMARY);
> > +
> > +			igt_plane_set_fb(primary, NULL);
> > +			igt_display_commit2(display,
> > COMMIT_ATOMIC);
> > +			igt_remove_fb(data.drm_fd, &data.fbs[crtc-
> > >crtc_index].red);
> > +			igt_remove_fb(data.drm_fd, &data.fbs[crtc-
> > >crtc_index].green);
> > +		}
> > +
> > +		igt_create_color_fb(data.drm_fd, mode->hdisplay,
> > mode->vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > DRM_FORMAT_MOD_LINEAR,
> > +				    1.f, 0.f, 0.f, &data.fbs[crtc-
> > >crtc_index].red);
> > +		igt_create_color_fb(data.drm_fd, mode->hdisplay,
> > mode->vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > DRM_FORMAT_MOD_LINEAR,
> > +				    0.f, 1.f, 0.f, &data.fbs[crtc-
> > >crtc_index].green);
> > +
> 
> Do we really need this recreation here before we even tried 
> 'igt_override_all_active_output_modes_to_fit_bw'? Because I'm
> thinking 
> that the mode we get here should be the same as we got earlier in the
> create_fbs?
> 

Hi Karthik,

Thank you for the review! You're absolutely correct.

The framebuffer recreation here is unnecessary since the FB created in
create_fbs() already has the correct dimensions for this output/CRTC
combination. Although we will test non-MST before MST test case, the
physical output connection and CRTC relationship won't change until
here.

I will fix this in the next version by removing this and directly using
the existing framebuffers.

Regards,
Jason-JH Lin

> Regards,
> Karthik.B.S