[Intel-gfx] [PATCH v2 08/13] drm/i915: Clean up skl+ vs. icl+ watermark computation

[Matt Roper]

On Wed, Nov 21, 2018 at 09:05:33PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 20, 2018 at 02:44:34PM -0800, Matt Roper wrote:
> > On Wed, Nov 14, 2018 at 11:07:24PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
...snip...
> > > +static int icl_build_plane_wm(struct skl_ddb_allocation *ddb,
> > > +			      struct skl_pipe_wm *pipe_wm,
> > > +			      struct intel_crtc_state *crtc_state,
> > > +			      const struct intel_plane_state *plane_state)
> > > +{
> > > +	enum plane_id plane_id = to_intel_plane(plane_state->base.plane)->id;
> > > +	int ret;
> > > +
> > > +	/* Watermarks calculated in master */
> > > +	if (plane_state->slave)
> > > +		return 0;
> > > +
> > > +	if (plane_state->linked_plane) {
> > > +		const struct drm_framebuffer *fb = plane_state->base.fb;
> > > +		enum plane_id y_plane_id = plane_state->linked_plane->id;
> > > +
> > > +		WARN_ON(!fb->format->is_yuv ||
> > > +			fb->format->num_planes == 1);
> > > +
> > > +		ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state,
> > > +						y_plane_id, 0);
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		ret = skl_build_plane_wm_single(ddb, crtc_state, plane_state,
> > > +						plane_id, 1);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else if (intel_wm_plane_visible(crtc_state, plane_state)) {
> > 
> > Isn't a visibility test also relevant to the nv12 (master plane) case
> > above?  I don't understand why we'd only test it for rgb planes.
> 
> linked_plane!=NULL implies that the plane is visible (see 
> icl_check_nv12_planes()). I should probably add another WARN_ON() for
> that.

Ah, okay.  In that case, with or without the WARN_ON(),

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795