From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755007AbcE3Nuk (ORCPT ); Mon, 30 May 2016 09:50:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44763 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754653AbcE3Nui convert rfc822-to-8bit (ORCPT ); Mon, 30 May 2016 09:50:38 -0400 Message-ID: <1464616236.5179.41.camel@redhat.com> Subject: Re: [PATCH] Add virtio gpu driver. From: Gerd Hoffmann To: Daniel Vetter Cc: virtio-dev@lists.oasis-open.org, "Michael S. Tsirkin" , "open list:ABI/API" , Rusty Russell , open list , "open list:DRM DRIVERS" , "open list:VIRTIO CORE, NET..." , Dave Airlie Date: Mon, 30 May 2016 15:50:36 +0200 In-Reply-To: <20160527090313.GX27098@phenom.ffwll.local> References: <1427213239-8775-1-git-send-email-kraxel@redhat.com> <20150324165057.GN1349@phenom.ffwll.local> <1427718227.3372.33.camel@nilsson.home.kraxel.org> <20150330144927.GN23521@phenom.ffwll.local> <1464335302.10663.10.camel@redhat.com> <20160527090313.GX27098@phenom.ffwll.local> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Mime-Version: 1.0 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 30 May 2016 13:50:38 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, > - add a small core function to registerr HOT_X/HOT_Y for a (cursor) plane, > e.g. drm_plane_register_hotspot(). That should allocate the properties > (if they don't exist yet) and then attach those props to the cursor. We > don't want those props everywhere, but only on drivers that support/need > them, aka virtual hw. Hmm, why is this special to virtual hw? > if (crtc->cursor) { > - ret = drm_mode_cursor_universal(crtc, req, file_priv); > + if (drm_core_check_feature(DRIVER_ATOMIC)) > + ret = drm_mode_cursor_atomic(crtc, req, file_priv); > + else > + ret = drm_mode_cursor_universal(crtc, req, file_priv); > goto out; > drm_mode_cursor_atomic would simply be a fusing of > drm_mode_cursor_universal + drm_atomic_helper_update_plane (dump all the > intermediate variables and store directly in the plane state), with the > addition of also storing hot_x/y into the plane state. Hmm, that'll either make drm_mode_cursor_atomic a big cut+pasted function, or need quite some refactoring to move common code into functions callable from both drm_mode_cursor_atomic +drm_mode_cursor_universal ... Why attach the hotspot to the plane? Wouldn't it make more sense to make it a framebuffer property? cheers, Gerd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gerd Hoffmann Subject: Re: [PATCH] Add virtio gpu driver. Date: Mon, 30 May 2016 15:50:36 +0200 Message-ID: <1464616236.5179.41.camel@redhat.com> References: <1427213239-8775-1-git-send-email-kraxel@redhat.com> <20150324165057.GN1349@phenom.ffwll.local> <1427718227.3372.33.camel@nilsson.home.kraxel.org> <20150330144927.GN23521@phenom.ffwll.local> <1464335302.10663.10.camel@redhat.com> <20160527090313.GX27098@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <20160527090313.GX27098@phenom.ffwll.local> Sender: linux-kernel-owner@vger.kernel.org To: Daniel Vetter Cc: virtio-dev@lists.oasis-open.org, "Michael S. Tsirkin" , "open list:ABI/API" , Rusty Russell , open list , "open list:DRM DRIVERS" , "open list:VIRTIO CORE, NET..." , Dave Airlie List-Id: linux-api@vger.kernel.org Hi, > - add a small core function to registerr HOT_X/HOT_Y for a (cursor) plane, > e.g. drm_plane_register_hotspot(). That should allocate the properties > (if they don't exist yet) and then attach those props to the cursor. We > don't want those props everywhere, but only on drivers that support/need > them, aka virtual hw. Hmm, why is this special to virtual hw? > if (crtc->cursor) { > - ret = drm_mode_cursor_universal(crtc, req, file_priv); > + if (drm_core_check_feature(DRIVER_ATOMIC)) > + ret = drm_mode_cursor_atomic(crtc, req, file_priv); > + else > + ret = drm_mode_cursor_universal(crtc, req, file_priv); > goto out; > drm_mode_cursor_atomic would simply be a fusing of > drm_mode_cursor_universal + drm_atomic_helper_update_plane (dump all the > intermediate variables and store directly in the plane state), with the > addition of also storing hot_x/y into the plane state. Hmm, that'll either make drm_mode_cursor_atomic a big cut+pasted function, or need quite some refactoring to move common code into functions callable from both drm_mode_cursor_atomic +drm_mode_cursor_universal ... Why attach the hotspot to the plane? Wouldn't it make more sense to make it a framebuffer property? cheers, Gerd