[PATCH v5 2/4] drm: introduce drm_writeback_connector_init_with_encoder() API
Liviu Dudau
liviu.dudau at arm.com
Thu Mar 31 10:01:30 UTC 2022
On Fri, Mar 25, 2022 at 09:31:35AM -0700, Abhinav Kumar wrote:
> Hi Liviu
Hi Abhinav,
Sorry for the delay, got busy with other things at the beginning of the week.
>
> On 3/25/2022 3:19 AM, Liviu Dudau wrote:
> > On Thu, Mar 24, 2022 at 09:36:50AM -0700, Abhinav Kumar wrote:
> > > Hi Liviu
> >
> > Hello,
> >
> > >
> > > Thanks for the response.
> > >
> > > On 3/24/2022 3:12 AM, Liviu Dudau wrote:
> > > > On Wed, Mar 23, 2022 at 11:28:56AM -0700, Abhinav Kumar wrote:
> > > > > Hi Liviu
> > > >
> > > > Hello,
> > > >
> > > > >
> > > > > Thanks for the review.
> > > > >
> > > > > On 3/23/2022 9:46 AM, Liviu Dudau wrote:
> > > > > > On Mon, Mar 21, 2022 at 04:56:43PM -0700, Abhinav Kumar wrote:
> > > > > > > For vendors drivers which pass an already allocated and
> > > > > > > initialized encoder especially for cases where the encoder
> > > > > > > hardware is shared OR the writeback encoder shares the resources
> > > > > > > with the rest of the display pipeline introduce a new API,
> > > > > > > drm_writeback_connector_init_with_encoder() which expects
> > > > > > > an initialized encoder as a parameter and only sets up the
> > > > > > > writeback connector.
> > > > > > >
> > > > > > > changes in v5:
> > > > > > > - reorder this change to come before in the series
> > > > > > > to avoid incorrect functionality in subsequent changes
> > > > > > > - continue using struct drm_encoder instead of
> > > > > > > struct drm_encoder * and switch it in next change
> > > > > > >
> > > > > > > Signed-off-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
> > > > > >
> > > > > > Hi Abhinav,
> > > > > >
> > > > > > > ---
> > > > > > > drivers/gpu/drm/drm_writeback.c | 143 ++++++++++++++++++++++++++++------------
> > > > > > > include/drm/drm_writeback.h | 5 ++
> > > > > > > 2 files changed, 106 insertions(+), 42 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c
> > > > > > > index dc2ef12..abe78b9 100644
> > > > > > > --- a/drivers/gpu/drm/drm_writeback.c
> > > > > > > +++ b/drivers/gpu/drm/drm_writeback.c
> > > > > > > @@ -149,37 +149,15 @@ static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {
> > > > > > > .destroy = drm_encoder_cleanup,
> > > > > > > };
> > > > > > > -/**
> > > > > > > - * drm_writeback_connector_init - Initialize a writeback connector and its properties
> > > > > > > - * @dev: DRM device
> > > > > > > - * @wb_connector: Writeback connector to initialize
> > > > > > > - * @con_funcs: Connector funcs vtable
> > > > > > > - * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
> > > > > > > - * @formats: Array of supported pixel formats for the writeback engine
> > > > > > > - * @n_formats: Length of the formats array
> > > > > > > - * @possible_crtcs: possible crtcs for the internal writeback encoder
> > > > > > > - *
> > > > > > > - * This function creates the writeback-connector-specific properties if they
> > > > > > > - * have not been already created, initializes the connector as
> > > > > > > - * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> > > > > > > - * values. It will also create an internal encoder associated with the
> > > > > > > - * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
> > > > > > > - * the encoder helper.
> > > > > > > - *
> > > > > > > - * Drivers should always use this function instead of drm_connector_init() to
> > > > > > > - * set up writeback connectors.
> > > > > > > - *
> > > > > > > - * Returns: 0 on success, or a negative error code
> > > > > > > - */
> > > > > > > -int drm_writeback_connector_init(struct drm_device *dev,
> > > > > > > - struct drm_writeback_connector *wb_connector,
> > > > > > > - const struct drm_connector_funcs *con_funcs,
> > > > > > > - const struct drm_encoder_helper_funcs *enc_helper_funcs,
> > > > > > > - const u32 *formats, int n_formats, uint32_t possible_crtcs)
> > > > > > > +static int drm_writeback_connector_setup(struct drm_device *dev,
> > > > > > > + struct drm_writeback_connector *wb_connector,
> > > > > > > + const struct drm_connector_funcs *con_funcs, const u32 *formats,
> > > > > > > + int n_formats)
> > > > > > > {
> > > > > > > struct drm_property_blob *blob;
> > > > > > > - struct drm_connector *connector = &wb_connector->base;
> > > > > > > struct drm_mode_config *config = &dev->mode_config;
> > > > > > > + struct drm_connector *connector = &wb_connector->base;
> > > > > > > +
> > > > > >
> > > > > > Point of this reordering the statements is...?
> > > > > This diff can be avoided. There was no reason to reorder this. I will remove
> > > > > this re-order.
> > > > > >
> > > > > > > int ret = create_writeback_properties(dev);
> > > > > > > if (ret != 0)
> > > > > > > @@ -187,18 +165,10 @@ int drm_writeback_connector_init(struct drm_device *dev,
> > > > > > > blob = drm_property_create_blob(dev, n_formats * sizeof(*formats),
> > > > > > > formats);
> > > > > > > - if (IS_ERR(blob))
> > > > > > > - return PTR_ERR(blob);
> > > > > > > -
> > > > > > > - drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> > > > > > > -
> > > > > > > - wb_connector->encoder.possible_crtcs = possible_crtcs;
> > > > > > > -
> > > > > > > - ret = drm_encoder_init(dev, &wb_connector->encoder,
> > > > > > > - &drm_writeback_encoder_funcs,
> > > > > > > - DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > > > > > - if (ret)
> > > > > > > - goto fail;
> > > > > > > + if (IS_ERR(blob)) {
> > > > > > > + ret = PTR_ERR(blob);
> > > > > > > + return ret;
> > > > > > > + }
> > > > > >
> > > > > > I don't see why you have changed the earlier code to store the result of PTR_ERR into
> > > > > > ret other than to help your debugging. I suggest that you keep the existing code that
> > > > > > returns PTR_ERR(blob) directly and you will have a nicer diff stat as well.
> > > > > Sure, i can fix this for a smaller diff stat.
> > > > > >
> > > > > > > connector->interlace_allowed = 0;
> > > > > > > @@ -237,13 +207,102 @@ int drm_writeback_connector_init(struct drm_device *dev,
> > > > > > > attach_fail:
> > > > > > > drm_connector_cleanup(connector);
> > > > > > > connector_fail:
> > > > > > > - drm_encoder_cleanup(&wb_connector->encoder);
> > > > > > > -fail:
> > > > > > > drm_property_blob_put(blob);
> > > > > > > return ret;
> > > > > > > }
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * drm_writeback_connector_init - Initialize a writeback connector and its properties
> > > > > > > + * @dev: DRM device
> > > > > > > + * @wb_connector: Writeback connector to initialize
> > > > > > > + * @con_funcs: Connector funcs vtable
> > > > > > > + * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
> > > > > > > + * @formats: Array of supported pixel formats for the writeback engine
> > > > > > > + * @n_formats: Length of the formats array
> > > > > > > + * @possible_crtcs: possible crtcs for the internal writeback encoder
> > > > > > > + *
> > > > > > > + * This function creates the writeback-connector-specific properties if they
> > > > > > > + * have not been already created, initializes the connector as
> > > > > > > + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> > > > > > > + * values. It will also create an internal encoder associated with the
> > > > > > > + * drm_writeback_connector and set it to use the @enc_helper_funcs vtable for
> > > > > > > + * the encoder helper.
> > > > > > > + *
> > > > > > > + * Drivers should always use this function instead of drm_connector_init() to
> > > > > > > + * set up writeback connectors.
> > > > > > > + *
> > > > > > > + * Returns: 0 on success, or a negative error code
> > > > > > > + */
> > > > > > > +int drm_writeback_connector_init(struct drm_device *dev,
> > > > > > > + struct drm_writeback_connector *wb_connector,
> > > > > > > + const struct drm_connector_funcs *con_funcs,
> > > > > > > + const struct drm_encoder_helper_funcs *enc_helper_funcs,
> > > > > > > + const u32 *formats, int n_formats, uint32_t possible_crtcs)
> > > > > > > +{
> > > > > > > + int ret = 0;
> > > > > > > +
> > > > > > > + drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
> > > > > > > +
> > > > > > > + wb_connector->encoder.possible_crtcs = possible_crtcs;
> > > > > > > +
> > > > > > > + ret = drm_encoder_init(dev, &wb_connector->encoder,
> > > > > > > + &drm_writeback_encoder_funcs,
> > > > > > > + DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > > > > > + if (ret)
> > > > > > > + return ret;
> > > > > > > +
> > > > > > > + ret = drm_writeback_connector_setup(dev, wb_connector, con_funcs, formats,
> > > > > > > + n_formats);
> > > > > > > +
> > > > > > > + if (ret)
> > > > > > > + drm_encoder_cleanup(&wb_connector->encoder);
> > > > > > > +
> > > > > > > + return ret;
> > > > > > > +}
> > > > > > > EXPORT_SYMBOL(drm_writeback_connector_init);
> > > > > > > +/**
> > > > > > > + * drm_writeback_connector_init_with_encoder - Initialize a writeback connector and its properties
> > > > > > > + * using the encoder which already assigned and initialized
> > > > > > > + *
> > > > > > > + * @dev: DRM device
> > > > > > > + * @wb_connector: Writeback connector to initialize
> > > > > > > + * @con_funcs: Connector funcs vtable
> > > > > > > + * @enc_helper_funcs: Encoder helper funcs vtable to be used by the internal encoder
This is the encoder helper funcs that I was talking about. Maybe it is better if you
send a new, cleaner version and we can continue the conversation/review there?
Best regards,
Liviu
> > > > > > > + * @formats: Array of supported pixel formats for the writeback engine
> > > > > > > + * @n_formats: Length of the formats array
> > > > > > > + *
> > > > > > > + * This function creates the writeback-connector-specific properties if they
> > > > > > > + * have not been already created, initializes the connector as
> > > > > > > + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property
> > > > > > > + * values.
> > > > > > > + *
> > > > > > > + * This function assumes that the drm_writebac_connector's encoder has already been
> > > > > >
> > > > > > spelling: writeback
> > > > > Ack. will fix this.
> > > > > >
> > > > > > > + * created and initialized before invoking this function.
> > > > > > > + *
> > > > > > > + * In addition, this function also assumes that callers of this API will manage
> > > > > > > + * assigning the encoder helper functions, possible_crtcs and any other encoder
> > > > > > > + * specific operation which is otherwise handled by drm_writeback_connector_init().
> > > > > > > + *
> > > > > > > + * Drivers should always use this function instead of drm_connector_init() to
> > > > > > > + * set up writeback connectors.
> > > > > >
> > > > > > .... if they want to manage themselves the lifetime of the associated encoder.
> > > > > >
> > > > > > We're not trying to replace drm_writeback_connector_init() function here, only to
> > > > > > provide an alternative function to call for special cases.
> > > > >
> > > > > Yes, we are trying to provide an alternative function call for special case
> > > > > where the encoder is a shared encoder and/or where the resources of the
> > > > > writeback encoder are shared with other hardware blocks.
> > > > >
> > > > > I can add that comment to this function's doc.
> > > > >
> > > > > >
> > > > > > > + *
> > > > > > > + * Returns: 0 on success, or a negative error code
> > > > > > > + */
> > > > > > > +int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > > > > > + struct drm_writeback_connector *wb_connector,
> > > > > > > + const struct drm_connector_funcs *con_funcs, const u32 *formats,
> > > > > > > + int n_formats)
> > > > > >
> > > > > > Where is the encoder parameter? Isn't that the reason why the function is called
> > > > > > `_with_encoder`?
> > > > > The encoder parameter is skipped here because this function assumes that
> > > > > wb_connector->encoder has already been initialized, setup with functions and
> > > > > its possible_crts have already been set prior to calling this function like
> > > > > the vc4 example shows. So this function doesnt need an explicit encoder
> > > > > parameter. Let me know if thats a concern.
> > > > > >
> > > > > > I think there might have been too many version of these patchsets and things are
> > > > > > starting to be confusing. Please revisit the series without rushing and come up with
> > > > > > a plan of action. My understanding of watching this series has been that you're
> > > > > > trying to come up with a function that does *connector* initialisation but skips the
> > > > > > *encoder* initialisation because it might have been already done by the driver. The
> > > > > > idea will be then to have a function `drm_writeback_connector_init_with_encoder()`
> > > > > > that does *all* the work that `drm_writeback_connector_init()` does at the moment minus
> > > > > > the encoder initialisation part. Then `drm_writeback_connector_init()` only
> > > > > > initialises the internal encoder and calls `drm_writeback_connector_init_with_encoder()`.
> > > > > > There is no need to have the `drm_writeback_connector_setup()` function at all.
> > > > > >
> > > > > > Best regards,
> > > > > > Liviu
> > > > > >
> > > > >
> > > > > I agree there have been a 4 revisions prior to this because of the way this
> > > > > affects the existing writeback drivers. So initial revision was a bit
> > > > > intrusive into other drivers (which was just mostly a take over from the
> > > > > previous patchset posted by the Co-developer ) and since rev3 we have
> > > > > decided to have a separate API drm_writeback_connector_init_with_encoder()
> > > > > so that existing clients are unaffected and it works seamlessly under the
> > > > > hood.
> > > > >
> > > > > Only clients which already embed an encoder (vc4) and the new ones which
> > > > > have special encoder requirements like the MSM driver for which I am
> > > > > preparing these changes for will use the new API.
> > > > >
> > > > > Apologies for the revisions, but thanks to some great feedback from you and
> > > > > laurent its shaping up nicely and reaching its conclusion I feel.
> > > >
> > > > I think it is only natural that there will be some iterations. If I remember
> > > > correctly, the initial writeback series has something like 13 revisions :)
> > > >
> > > > >
> > > > > So i think this revision is pretty close to being clean thanks to the
> > > > > feedback you gave on rev4. Between rev4 and this one I didnt drastically
> > > > > change the design other than re-ordering the changes to avoid the
> > > > > intermediate patches having an incorrect state for the vc4 encoder. So all
> > > > > your comments related to the encoder_cleanup() and vc4's encoder not getting
> > > > > initialized would have gotten addressed but overall concept remained same.
> > > > >
> > > > > You are right, we are trying to come up with a function which does connector
> > > > > initialization but skips the encoder part because that has already been done
> > > > > in the client side of this API ( hence _with_encoder() name ). The
> > > > > "_with_encoder" indicates that the caller already has an encoder for the
> > > > > writeback connector which is being passed so there is no need to pass the
> > > > > encoder again.
> > > > >
> > > > > I thought this addresses all the concerns posted in the previous series.
> > > > >
> > > > > So are you suggesting something like below?
> > > > >
> > > > > 1) rename drm_writeback_connector_setup() to
> > > > > drm_writeback_connector_init_with_encoder()
> > > > > (essentially thats what will end up happening )
> > > >
> > > > Correct. Without the pointless reordering of code it should be about 3 lines of code
> > > > that get removed (the calls to drm_encoder_helper_add() and drm_encoder_init()).
> > > >
> > >
> > > But isnt thats how it already looks.
> > >
> > > int drm_writeback_connector_init(struct drm_device *dev,
> > > struct drm_writeback_connector *wb_connector,
> > > const struct drm_connector_funcs *con_funcs,
> > > const struct drm_encoder_helper_funcs *enc_helper_funcs,
> > > const u32 *formats, int n_formats, uint32_t possible_crtcs)
> > > {
> > > int ret = 0;
> > >
> > > wb_connector->encoder = &wb_connector->internal_encoder;
> > >
> > > drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> > >
> > > wb_connector->encoder->possible_crtcs = possible_crtcs;
> > >
> > > ret = drm_encoder_init(dev, wb_connector->encoder,
> > > &drm_writeback_encoder_funcs,
> > > DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > if (ret)
> > > return ret;
> > >
> > > ret = drm_writeback_connector_setup(dev, wb_connector, con_funcs,
> > > formats,
> > > n_formats);
> > >
> > > So the only change you are requesting is that, instead of having a new
> > > drm_writeback_connector_setup(), just call
> > > drm_writeback_connector_init_with_encoder().
> > >
> > > It will essentially look like
> > >
> > > int drm_writeback_connector_init(struct drm_device *dev,
> > > struct drm_writeback_connector *wb_connector,
> > > const struct drm_connector_funcs *con_funcs,
> > > const struct drm_encoder_helper_funcs *enc_helper_funcs,
> > > const u32 *formats, int n_formats, uint32_t possible_crtcs)
> > > {
> > > int ret = 0;
> > >
> > > wb_connector->encoder = &wb_connector->internal_encoder;
> >
> > (1)
> >
> > >
> > > drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
> > >
> > > wb_connector->encoder->possible_crtcs = possible_crtcs;
> > >
> > > ret = drm_encoder_init(dev, wb_connector->encoder,
> > > &drm_writeback_encoder_funcs,
> > > DRM_MODE_ENCODER_VIRTUAL, NULL);
> > > if (ret)
> > > return ret;
> > >
> > > ret = drm_writeback_connector_init_with_encoder(dev, wb_connector,
> > > con_funcs, formats,
> > > n_formats);
> >
> > Yes, this is exactly what I had in mind.
>
> Thank you for confirming.
>
> >
> >
> > >
> > > drm_writeback_connector_init_with_encoder() will still not have an encoder
> > > parameter because of what I wrote previously.
> >
> > But in your patch drm_writeback_connector_init_with_encoder() still has an
> > encoder_funcs pointer which is useless, as the expectations are (AFAII) that the
> > whole encoder init dance has already happened. And while you have a point that you
> > can set the encoder pointer in the connector before calling
> > drm_writeback_connector_init_with_encoder() I think it would be easier to read if you
> > pass the encoder explicitly in the parameter list and skip the assignment in (1) and
> > do it inside drm_writeback_connector_init_with_encoder(). Again, your code is not
> > wrong, I just think we should be explicit so that code is easier to read.
>
> Sorry, but I am missing something here.
>
> Where is the encoder_funcs pointer in
> drm_writeback_connector_init_with_encoder()? It only has drm_connector_funcs
> pointer. Because, like I mentioned in the earlier comment, the callers of
> this API would have already setup the encoder with the required
> functions/possible_crtcs etc.
>
> int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> struct drm_writeback_connector *wb_connector,
> const struct drm_connector_funcs *con_funcs, const u32 *formats,
> int n_formats)
> {
> int ret = 0;
>
> ret = drm_writeback_connector_setup(dev, wb_connector, con_funcs,
> formats,
> n_formats);
>
> return ret;
> }
>
>
> So are you suggesting I pass the drm_encoder as a parameter to
> drm_writeback_connector_init_with_encoder as well?
>
> That has two potential issues:
>
> 1) If we move just the assignment of wb_connector->encoder to inside
> drm_writeback_connector_init_with_encoder, we cannot do these before calling
> drm_writeback_connector_init_with_encoder() as wb_connector->encoder will be
> NULL for the cases which use internal_encoder
>
> drm_encoder_helper_add(wb_connector->encoder, enc_helper_funcs);
>
> wb_connector->encoder->possible_crtcs = possible_crtcs;
>
> ret = drm_encoder_init(dev, wb_connector->encoder,
> &drm_writeback_encoder_funcs,
> DRM_MODE_ENCODER_VIRTUAL, NULL);
Where can you not do this? Inside drm_writeback_connector_init_with_encoder() you
don't need to do that, as the function is intended to initialise a writeback
connector using a provided encoder that has been already initialised. Inside
drm_writeback_connector_init() you will call these functions using the internal
encoder pointer that you have added in the series.
>
> 2) We can only do these changes once wb_connector->encoder has been changed
> to a pointer which happens in patch 4 of this series
> https://patchwork.freedesktop.org/patch/479084/?series=101260&rev=5
>
> I was told to split the functionality of adding the new API in a separate
> patch by laurent.
>
> To keep changes independent and functionally correct I thought this is the
> best split.
>
> So the cleanup you are suggesting can be done in patch 4 of this series but
> still I am not sure how to get around concern (1) because the encoder has to
> be fully initialized before attaching to a wb_connector.
I think if you keep the vc4 patch in patch 4 and move the rest in here it will make
more sense. Again, I think a new series cleaned up might help others to review as
well (specially Laurent).
Best regards,
Liviu
>
>
> >
> > >
> > > Hope this is what you had in mind as well.
> > >
> > > > >
> > > > > 2) Inside drm_writeback_connector_init() check:
> > > > >
> > > > > drm_writeback_connector_init(.....)
> > > > > {
> > > > > if (!wb_conn->encoder)
> > > > > initialize the encoder
> > > >
> > > > No, the assumption of drm_writeback_connector_init() is that we will provide the
> > > > encoder, so no need to check that one is already provided. What you could do is:
> > > >
> > > > WARN_ON(wb_conn->encoder);
> > > >
> > >
> > > Got it, i will add a warning inside drm_writeback_connector_init() to
> > > emphasize that its only meant for cases where an encoder is not provided.
> > >
> > > > before we overwrite the encoder. That way we will get a nice warning in the kernel
> > > > log if someone tries to call drm_writeback_connector_init() with a preset encoder.
> > > >
> > > > >
> > > > > call drm_writeback_**_init_with_encoder()
> > > > > }
> > > > >
> > > > > This will also work but have the foll concerns/questions:
> > > > >
> > > > > 1) Will drm_writeback_connector_init_with_encoder() be exported if we change
> > > > > as per your suggestion or will all clients continue to use
> > > > > drm_writeback_connector_init() only? We wanted to have a separate function
> > > > > for new clients.
> > > >
> > > > Yes, we will need to export drm_writeback_connector_init_with_encoder() as well.
> > > >
> > > Alright, so vc4 and new vendors which provide their own encoder will use
> > > this one. So no changes to the rest of the series.
> > >
> > > > >
> > > > > 2) How will we detect that encoder needs to be initialized without it being
> > > > > a pointer which happens in the later change. So ordering becomes an issue.
> > > >
> > > > I think the init problem is simple. You either call drm_writeback_connector_init()
> > > > and the framework provides you with an encoder, or you call
> > > > drm_writeback_connector_init_with_encoder() and the framework will use yours. The
> > > > problems will show up on the cleanup and exit codes, where we need to be able to skip
> > > > the cleanup if the encoder pointer is not the internal one. I think a simple
> > > >
> > > > if (connector->encoder == &connector->internal_encoder)
> > > > do_encoder_cleanup_work_here()
> > > >
> > > > should work.
> > >
> > > Sorry, I am missing something here.
> > >
> > > Even in this current patch, the drm_encoder_cleanup() is done only inside
> > > drm_writeback_connector_init() where internal_encoder is used.
> > >
> > > For drm_writeback_connector_init_with_encoder(), we dont do the cleanup and
> > > expect the caller to do it like vc4 does in the next patch.
> > >
> > > So why do we need such a condition?
> >
> > You're right. I was thinking that at cleanup time we also need to do work with the
> > right encoder, but that should accomplished by passing the right .destroy hook in the
> > drm_encoder_funcs pointer via drm_encoder_init. So if the special drivers to the
> > initialisation correctly it should all work fine, please disregard my comments.
> >
> > Best regards,
> > Liviu
> >
> >
> > >
> > > >
> > > > >
> > > > > Thats why I thought this is the best way to address the comments and keep
> > > > > the functionality intact with each change.
> > > > >
> > > > > Let me know if I have misunderstood some comment or idea.
> > > >
> > > > I hope that with these clarifications we are both on the same page.
> > > >
> > > > Best regards,
> > > > Liviu
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > > +{
> > > > > > > + int ret = 0;
> > > > > > > +
> > > > > > > + ret = drm_writeback_connector_setup(dev, wb_connector, con_funcs, formats,
> > > > > > > + n_formats);
> > > > > > > +
> > > > > > > + return ret;
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL(drm_writeback_connector_init_with_encoder);
> > > > > > > +
> > > > > > > int drm_writeback_set_fb(struct drm_connector_state *conn_state,
> > > > > > > struct drm_framebuffer *fb)
> > > > > > > {
> > > > > > > diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h
> > > > > > > index db6214f..0093bab 100644
> > > > > > > --- a/include/drm/drm_writeback.h
> > > > > > > +++ b/include/drm/drm_writeback.h
> > > > > > > @@ -152,6 +152,11 @@ int drm_writeback_connector_init(struct drm_device *dev,
> > > > > > > const struct drm_encoder_helper_funcs *enc_helper_funcs,
> > > > > > > const u32 *formats, int n_formats, uint32_t possible_crtcs);
> > > > > > > +int drm_writeback_connector_init_with_encoder(struct drm_device *dev,
> > > > > > > + struct drm_writeback_connector *wb_connector,
> > > > > > > + const struct drm_connector_funcs *con_funcs, const u32 *formats,
> > > > > > > + int n_formats);
> > > > > > > +
> > > > > > > int drm_writeback_set_fb(struct drm_connector_state *conn_state,
> > > > > > > struct drm_framebuffer *fb);
> > > > > > > --
> > > > > > > 2.7.4
> > > > > > >
> > > > > >
> > > >
> >
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
More information about the dri-devel
mailing list