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
