Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 17 Sep 2019 14:53:01 +0200
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Ayan Halder <Ayan.Halder@arm.com>
Cc:     Liviu Dudau <Liviu.Dudau@arm.com>,
        Brian Starkey <Brian.Starkey@arm.com>,
        "maarten.lankhorst@linux.intel.com" 
        <maarten.lankhorst@linux.intel.com>,
        "maxime.ripard@bootlin.com" <maxime.ripard@bootlin.com>,
        "sean@poorly.run" <sean@poorly.run>,
        "airlied@linux.ie" <airlied@linux.ie>,
        "daniel@ffwll.ch" <daniel@ffwll.ch>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        nd <nd@arm.com>
Subject: Re: [RFC PATCH] drm:- Add a modifier to denote 'protected'
 framebuffer
Message-ID: <20190917125301.GQ3958@phenom.ffwll.local>
Mail-Followup-To: Ayan Halder <Ayan.Halder@arm.com>,
        Liviu Dudau <Liviu.Dudau@arm.com>,
        Brian Starkey <Brian.Starkey@arm.com>,
        "maarten.lankhorst@linux.intel.com" <maarten.lankhorst@linux.intel.com>,
        "maxime.ripard@bootlin.com" <maxime.ripard@bootlin.com>,
        "sean@poorly.run" <sean@poorly.run>,
        "airlied@linux.ie" <airlied@linux.ie>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        nd <nd@arm.com>
References: <20190909134241.23297-1-ayan.halder@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190909134241.23297-1-ayan.halder@arm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Sep 09, 2019 at 01:42:53PM +0000, Ayan Halder wrote:
> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the framebuffer
> is allocated in a protected system memory.
> Essentially, we want to support EGL_EXT_protected_content in our komeda driver.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.halder@arm.com>
> 
> /-- Note to reviewer
> Komeda driver is capable of rendering DRM (Digital Rights Management) protected
> content. The DRM content is stored in a framebuffer allocated in system memory
> (which needs some special hardware signals for access).
> 
> Let us ignore how the protected system memory is allocated and for the scope of
> this discussion, we want to figure out the best way possible for the userspace
> to communicate to the drm driver to turn the protected mode on (for accessing the
> framebuffer with the DRM content) or off.
> 
> The possible ways by which the userspace could achieve this is via:-
> 
> 1. Modifiers :- This looks to me the best way by which the userspace can
> communicate to the kernel to turn the protected mode on for the komeda driver
> as it is going to access one of the protected framebuffers. The only problem is
> that the current modifiers describe the tiling/compression format. However, it
> does not hurt to extend the meaning of modifiers to denote other attributes of
> the framebuffer as well.
> 
> The other reason is that on Android, we get an info from Gralloc
> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This can
> be used to set up the modifier/s (AddFB2) during framebuffer creation.

How does this mesh with other modifiers, like AFBC? That's where I see the
issue here.
> 
> 2. Framebuffer flags :- As of today, this can be one of the two values
> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm
> framebuffer flags are generic to the drm subsystem and ideally we should not
> introduce any driver specific constraint/feature.
> 
> 3. Connector property:- I could see the following properties used for DRM
> protected content:-
> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by
> userspace to request the kernel protect future content communicated over
> the link". Clearly, we are not concerned with the protection attributes of the
> transmitter. So, we cannot use this property for our case.
> 
> 4. DRM plane property:- Again, we want to communicate that the framebuffer(which
> can be attached to any plane) is protected. So introducing a new plane property
> does not help.
> 
> 5. DRM crtc property:- For the same reason as above, introducing a new crtc
> property does not help.

6. Just track this as part of buffer allocation, i.e. I think it does
matter how you allocate these protected buffers. We could add a "is
protected buffer" flag at the dma_buf level for this.

So yeah for this stuff here I think we do want the full userspace side,
from allocator to rendering something into this protected buffers (no need
to also have the entire "decode a protected bitstream part" imo, since
that will freak people out). Unfortunately, in my experience, that kills
it for upstream :-/ But also in my experience of looking into this for
other gpu's, we really need to have the full picture here to make sure
we're not screwing this up.
-Daniel

> 
> --/
> 
> ---
>  include/uapi/drm/drm_fourcc.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..38e5e81d11fe 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,15 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>  
> +/*
> + * Protected framebuffer
> + *
> + * The framebuffer is allocated in a protected system memory which can be accessed
> + * via some special hardware signals from the dpu. This is used to support
> + * 'GRALLOC_USAGE_PROTECTED' in our framebuffer for EGL_EXT_protected_content.
> + */
> +#define DRM_FORMAT_MOD_ARM_PROTECTED	fourcc_mod_code(ARM, (1ULL << 55))
> +
>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch