[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6a16f0023b62beba4658677bebcc4786da1ea4be.camel@redhat.com>
Date: Sun, 21 Apr 2024 21:54:25 -0400
From: Lyude Paul <lyude@...hat.com>
To: Benno Lossin <benno.lossin@...ton.me>, dri-devel@...ts.freedesktop.org
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, Miguel Ojeda
<ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Wedson Almeida
Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...sung.com>,
Alice Ryhl <aliceryhl@...gle.com>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/4] WIP: drm: Introduce rvkms
On Wed, 2024-03-27 at 21:06 +0000, Benno Lossin wrote:
> On 22.03.24 23:03, Lyude Paul wrote:
> > diff --git a/drivers/gpu/drm/rvkms/connector.rs
> > b/drivers/gpu/drm/rvkms/connector.rs
> > new file mode 100644
> > index 0000000000000..40f84d38437ee
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rvkms/connector.rs
> > @@ -0,0 +1,55 @@
> > +// TODO: License and stuff
> > +// Contain's rvkms's drm_connector implementation
> > +
> > +use super::{RvkmsDriver, RvkmsDevice, MAX_RES, DEFAULT_RES};
> > +use kernel::{
> > + prelude::*,
> > + drm::{
> > + device::Device,
> > + kms::{
> > + connector::{self, ConnectorGuard},
> > + ModeConfigGuard
> > + }
> > + },
> > + prelude::*
> > +};
> > +use core::marker::PhantomPinned;
> > +
> > +#[pin_data]
> > +pub(crate) struct DriverConnector {
> > + #[pin]
> > + _p: PhantomPinned
> > +}
>
> This struct does not need to be annotated with `#[pin_data]`, this
> should just work:
>
> pub(crate) struct DriverConnector;
>
> > +
> > +pub(crate) type Connector = connector::Connector<DriverConnector>;
> > +
> > +impl connector::DriverConnector for DriverConnector {
> > + type Initializer = impl PinInit<Self, Error>;
> > +
> > + type State = ConnectorState;
> > +
> > + type Driver = RvkmsDriver;
> > +
> > + type Args = ();
> > +
> > + fn new(dev: &Device<Self::Driver>, args: Self::Args) ->
> > Self::Initializer {
>
> And then here just return `Self`.
>
> This works, since there is a blanket impl `PinInit<T, E> for T`.
>
> Looking at how you use this API, I am not sure if you actually need
> pin-init for the type that implements `DriverConnector`.
> Do you need to store eg `Mutex<T>` or something else that needs
> pin-init in here in a more complex driver?
Most likely yes - a lot of drivers have various private locks contained
within their subclassed mode objects. I'm not sure we will in rvkms's
connector since vkms doesn't really do much with connectors - but we at
a minimum be using pinned types (spinlocks and hrtimers) in our
DriverCrtc implementation once I've started implementing support for
vblanks[1]
[1]
https://www.kernel.org/doc/html/v6.9-rc5/gpu/drm-kms.html?highlight=vblank#vertical-blanking
In nova (the main reason I'm working on rvkms in the first place),
we'll definitely have locks in our connectors and possibly other types.
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Powered by blists - more mailing lists