From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751488AbcEIOqv (ORCPT ); Mon, 9 May 2016 10:46:51 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:33537 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750998AbcEIOqu (ORCPT ); Mon, 9 May 2016 10:46:50 -0400 Date: Mon, 9 May 2016 16:46:48 +0200 From: Daniel Vetter To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= Cc: dri-devel@lists.freedesktop.org, daniel@ffwll.ch, treding@nvidia.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] drm: Add helper for simple display pipeline Message-ID: <20160509144648.GA27098@phenom.ffwll.local> Mail-Followup-To: Noralf =?iso-8859-1?Q?Tr=F8nnes?= , dri-devel@lists.freedesktop.org, treding@nvidia.com, linux-kernel@vger.kernel.org References: <1462454674-2246-1-git-send-email-noralf@tronnes.org> <1462454674-2246-4-git-send-email-noralf@tronnes.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1462454674-2246-4-git-send-email-noralf@tronnes.org> X-Operating-System: Linux phenom 4.6.0-rc5+ User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 05, 2016 at 03:24:33PM +0200, Noralf Trønnes wrote: > Provides helper functions for drivers that have a simple display > pipeline. Plane, crtc and encoder are collapsed into one entity. > > Signed-off-by: Noralf Trønnes > +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *pstate) > +{ > + struct drm_simple_display_pipe *pipe; > + struct drm_crtc_state *cstate; > + > + pipe = container_of(plane, struct drm_simple_display_pipe, plane); > + if (!pipe->funcs || !pipe->funcs->check) > + return 0; > + > + cstate = drm_atomic_get_existing_crtc_state(pstate->state, > + &pipe->crtc); > + > + return pipe->funcs->check(pipe, pstate, cstate); > +} Ok one thing I've missed here is that for most drivers this is way too simple a check function, which means we'll end up with tons of duplicated code. Things which the drm core allows, but simple pipelines all don't really cope with: - plane scaling - disabling the plane without the crtc (i.e. scan out black) - plane not sized to fill the entire hactive/vactive There's a helper to do most of these checks for you - drm_plane_helper_check_update. I think it'd be good to place a call for that in here, before we call down into the driver's ->check callback. But ofc before we return 0; we want these checks always done. And catch all these things so that drivers never fall over this pitfall. Noticed while discussing tilcdc atomic patches, since tilcdc could probably use drm_simple_display_pipe too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch