On Sat, 9 Apr 2022 at 03:54, Abhinav Kumar quic_abhinavk@quicinc.com wrote:

Clients of drm_writeback_connector_init() initialize the possible_crtcs and then invoke the call to this API.

To simplify things, allow passing possible_crtcs as a parameter to drm_writeback_connector_init() and make changes to the other drm drivers to make them compatible with this change.

changes in v2: - split the changes according to their functionality

changes in v3: - allow passing possible_crtcs for existing users of drm_writeback_connector_init() - squash the vendor changes into the same commit so that each patch in the series can compile individually

changes in v4: - keep only changes related to possible_crtcs - add line breaks after ARRAY_SIZE - stop using temporary variables for possible_crtcs

changes in v5: - None

changes in v6: - None

changes in v7: - wrap long lines to match the coding style of existing drivers - Fix indentation and remove parenthesis where not needed - use u32 instead of uint32_t for possible_crtcs

Signed-off-by: Abhinav Kumar quic_abhinavk@quicinc.com Acked-by: Liviu Dudau liviu.dudau@arm.com

Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

---

drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c | 4 ++-- drivers/gpu/drm/arm/malidp_mw.c | 4 ++-- drivers/gpu/drm/drm_writeback.c | 7 ++++++- drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 4 ++-- drivers/gpu/drm/vc4/vc4_txp.c | 3 ++- drivers/gpu/drm/vkms/vkms_writeback.c | 4 ++-- include/drm/drm_writeback.h | 3 ++- 7 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c index e465cc4..ce4b760 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c @@ -155,7 +155,6 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms, kwb_conn->wb_layer = kcrtc->master->wb_layer;

    wb_conn = &kwb_conn->base;

•   wb_conn->encoder.possible_crtcs = BIT(drm_crtc_index(&kcrtc->base));
  
    formats = komeda_get_layer_fourcc_list(&mdev->fmt_tbl,
                                           kwb_conn->wb_layer->layer_type,
  

@@ -164,7 +163,8 @@ static int komeda_wb_connector_add(struct komeda_kms_dev *kms, err = drm_writeback_connector_init(&kms->base, wb_conn, &komeda_wb_connector_funcs, &komeda_wb_encoder_helper_funcs,

•                                      formats, n_formats);
  

•                                      formats, n_formats,
  

•                                      BIT(drm_crtc_index(&kcrtc->base)));
    komeda_put_fourcc_list(formats);
    if (err) {
            kfree(kwb_conn);
  

diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c index f5847a7..204c869 100644 --- a/drivers/gpu/drm/arm/malidp_mw.c +++ b/drivers/gpu/drm/arm/malidp_mw.c @@ -212,7 +212,6 @@ int malidp_mw_connector_init(struct drm_device *drm) if (!malidp->dev->hw->enable_memwrite) return 0;

•   malidp->mw_connector.encoder.possible_crtcs = 1 << drm_crtc_index(&malidp->crtc);
    drm_connector_helper_add(&malidp->mw_connector.base,
                             &malidp_mw_connector_helper_funcs);
  
  

@@ -223,7 +222,8 @@ int malidp_mw_connector_init(struct drm_device *drm) ret = drm_writeback_connector_init(drm, &malidp->mw_connector, &malidp_mw_connector_funcs, &malidp_mw_encoder_helper_funcs,

•                                      formats, n_formats);
  

•                                      formats, n_formats,
  

•                                      1 << drm_crtc_index(&malidp->crtc));
    kfree(formats);
    if (ret)
            return ret;
  

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index dccf4504..9e0b845 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -157,6 +157,7 @@ static const struct drm_encoder_funcs drm_writeback_encoder_funcs = {

• @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

@@ -174,7 +175,8 @@ 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)
  

•                            const u32 *formats, int n_formats,
  

•                            u32 possible_crtcs)
  

{ struct drm_property_blob *blob; struct drm_connector *connector = &wb_connector->base; @@ -190,6 +192,9 @@ int drm_writeback_connector_init(struct drm_device *dev, 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);
  

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c index c79d125..505a905 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c @@ -200,7 +200,6 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu, { struct drm_writeback_connector *wb_conn = &rcrtc->writeback;

•   wb_conn->encoder.possible_crtcs = 1 << drm_crtc_index(&rcrtc->crtc);
    drm_connector_helper_add(&wb_conn->base,
                             &rcar_du_wb_conn_helper_funcs);
  
  

@@ -208,7 +207,8 @@ int rcar_du_writeback_init(struct rcar_du_device *rcdu, &rcar_du_wb_conn_funcs, &rcar_du_wb_enc_helper_funcs, writeback_formats,

•                                       ARRAY_SIZE(writeback_formats));
  

•                                       ARRAY_SIZE(writeback_formats),
  

•                                       1 << drm_crtc_index(&rcrtc->crtc));
  

}

void rcar_du_writeback_setup(struct rcar_du_crtc *rcrtc, diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 9809ca3..3447eb6 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -497,7 +497,8 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) ret = drm_writeback_connector_init(drm, &txp->connector, &vc4_txp_connector_funcs, &vc4_txp_encoder_helper_funcs,

•                                      drm_fmts, ARRAY_SIZE(drm_fmts));
  

•                                      drm_fmts, ARRAY_SIZE(drm_fmts),
  

•                                      0);
    if (ret)
            return ret;
  
  

diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c index 8694227..6d01e55 100644 --- a/drivers/gpu/drm/vkms/vkms_writeback.c +++ b/drivers/gpu/drm/vkms/vkms_writeback.c @@ -140,12 +140,12 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev) { struct drm_writeback_connector *wb = &vkmsdev->output.wb_connector;

•   vkmsdev->output.wb_connector.encoder.possible_crtcs = 1;
    drm_connector_helper_add(&wb->base, &vkms_wb_conn_helper_funcs);
  
    return drm_writeback_connector_init(&vkmsdev->drm, wb,
                                        &vkms_wb_connector_funcs,
                                        &vkms_wb_encoder_helper_funcs,
                                        vkms_wb_formats,
  

•                                       ARRAY_SIZE(vkms_wb_formats));
  

•                                       ARRAY_SIZE(vkms_wb_formats),
  

•                                       1);
  

} diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index 9697d27..5d9263f 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -150,7 +150,8 @@ 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);
  

•                            const u32 *formats, int n_formats,
  

•                            u32 possible_crtcs);
  
  

int drm_writeback_set_fb(struct drm_connector_state *conn_state, struct drm_framebuffer *fb); -- 2.7.4

On Sat, 9 Apr 2022 at 03:54, Abhinav Kumar quic_abhinavk@quicinc.com 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 v4: - removed the possible_crtcs part

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

changes in v6: - remove drm_writeback_connector_setup() and instead directly call drm_writeback_connector_init_with_encoder() - fix a drm_writeback_connector typo and function doc which incorrectly shows that the function accepts enc_helper_funcs - pass encoder as a parameter explicitly to the new API for better readability

changes in v7: - fix the function doc slightly as suggested by Liviu

Reviewed-by: Liviu Dudau liviu.dudau@arm.com Signed-off-by: Abhinav Kumar quic_abhinavk@quicinc.com

Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

---

drivers/gpu/drm/drm_writeback.c | 72 +++++++++++++++++++++++++++++++++-------- include/drm/drm_writeback.h | 6 ++++ 2 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 9e0b845..92658ad 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -178,6 +178,62 @@ int drm_writeback_connector_init(struct drm_device *dev, const u32 *formats, int n_formats, u32 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_init_with_encoder(dev, wb_connector, &wb_connector->encoder,
  

•                   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
• • @enc: handle to the already initialized drm encoder
• • @con_funcs: Connector funcs vtable
• • @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_writeback_connector's encoder has already been
• • 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.
• • 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.
• • 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, struct drm_encoder *enc,
  

•           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; @@ -191,15 +247,6 @@ int drm_writeback_connector_init(struct drm_device *dev, 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;
  
    connector->interlace_allowed = 0;
  
  

@@ -208,8 +255,7 @@ int drm_writeback_connector_init(struct drm_device *dev, if (ret) goto connector_fail;

•   ret = drm_connector_attach_encoder(connector,
  

•                                           &wb_connector->encoder);
  

•   ret = drm_connector_attach_encoder(connector, enc);
    if (ret)
            goto attach_fail;
  
  

@@ -238,12 +284,10 @@ 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; } -EXPORT_SYMBOL(drm_writeback_connector_init); +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 5d9263f..bb306fa 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -153,6 +153,12 @@ int drm_writeback_connector_init(struct drm_device *dev, const u32 *formats, int n_formats, u32 possible_crtcs);

+int drm_writeback_connector_init_with_encoder(struct drm_device *dev,

•                           struct drm_writeback_connector *wb_connector,
  

•                           struct drm_encoder *enc,
  

•                           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

On Sat, 9 Apr 2022 at 03:54, Abhinav Kumar quic_abhinavk@quicinc.com wrote:

For some vendor driver implementations, display hardware can be shared between the encoder used for writeback and the physical display.

In addition resources such as clocks and interrupts can also be shared between writeback and the real encoder.

To accommodate such vendor drivers and hardware, allow real encoder to be passed for drm_writeback_connector.

For existing clients, drm_writeback_connector_init() will use an internal_encoder under the hood and hence no changes will be needed.

changes in v7: - move this change before the vc4 change in the series to minimize the changes to vendor drivers in drm core changes

Signed-off-by: Abhinav Kumar quic_abhinavk@quicinc.com

Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

---

drivers/gpu/drm/drm_writeback.c | 18 ++++++++++++------ drivers/gpu/drm/vc4/vc4_txp.c | 4 ++-- include/drm/drm_writeback.h | 22 ++++++++++++++++++++-- 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_writeback.c b/drivers/gpu/drm/drm_writeback.c index 92658ad..0538674 100644 --- a/drivers/gpu/drm/drm_writeback.c +++ b/drivers/gpu/drm/drm_writeback.c @@ -180,21 +180,21 @@ int drm_writeback_connector_init(struct drm_device *dev, { int ret = 0;

•   drm_encoder_helper_add(&wb_connector->encoder, enc_helper_funcs);
  

•   drm_encoder_helper_add(&wb_connector->internal_encoder, enc_helper_funcs);
  
  

•   wb_connector->encoder.possible_crtcs = possible_crtcs;
  

•   wb_connector->internal_encoder.possible_crtcs = possible_crtcs;
  
  

•   ret = drm_encoder_init(dev, &wb_connector->encoder,
  

•   ret = drm_encoder_init(dev, &wb_connector->internal_encoder,
                           &drm_writeback_encoder_funcs,
                           DRM_MODE_ENCODER_VIRTUAL, NULL);
    if (ret)
            return ret;
  
  

•   ret = drm_writeback_connector_init_with_encoder(dev, wb_connector, &wb_connector->encoder,
  

•                   con_funcs, formats, n_formats);
  

•   ret = drm_writeback_connector_init_with_encoder(dev, wb_connector,
  

•                   &wb_connector->internal_encoder, con_funcs, formats, n_formats);
  
    if (ret)
  

•           drm_encoder_cleanup(&wb_connector->encoder);
  

•           drm_encoder_cleanup(&wb_connector->internal_encoder);
  
    return ret;
  

} @@ -239,6 +239,12 @@ int drm_writeback_connector_init_with_encoder(struct drm_device *dev, struct drm_mode_config *config = &dev->mode_config; int ret = create_writeback_properties(dev);

•   /*
  

•    * Assign the encoder passed to this API to the wb_connector's encoder.
  

•    * For drm_writeback_connector_init(), this shall be the internal_encoder
  

•    */
  

•   wb_connector->encoder = enc;
  

•   if (ret != 0)
            return ret;
  
  

diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c index 3447eb6..7e063a9 100644 --- a/drivers/gpu/drm/vc4/vc4_txp.c +++ b/drivers/gpu/drm/vc4/vc4_txp.c @@ -159,7 +159,7 @@ struct vc4_txp {

static inline struct vc4_txp *encoder_to_vc4_txp(struct drm_encoder *encoder) {

•   return container_of(encoder, struct vc4_txp, connector.encoder);
  

•   return container_of(encoder, struct vc4_txp, connector.internal_encoder);
  

}

static inline struct vc4_txp *connector_to_vc4_txp(struct drm_connector *conn) @@ -507,7 +507,7 @@ static int vc4_txp_bind(struct device *dev, struct device *master, void *data) if (ret) return ret;

•   encoder = &txp->connector.encoder;
  

•   encoder = txp->connector.encoder;
    encoder->possible_crtcs = drm_crtc_mask(crtc);
  
    ret = devm_request_irq(dev, irq, vc4_txp_interrupt, 0,
  

diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h index bb306fa..3fbae9d 100644 --- a/include/drm/drm_writeback.h +++ b/include/drm/drm_writeback.h @@ -25,13 +25,31 @@ struct drm_writeback_connector { struct drm_connector base;

    /**

•    * @encoder: Internal encoder used by the connector to fulfill
  

•    * @encoder: handle to drm_encoder used by the connector to fulfill
     * the DRM framework requirements. The users of the
     * @drm_writeback_connector control the behaviour of the @encoder
     * by passing the @enc_funcs parameter to drm_writeback_connector_init()
     * function.
  

•    *
  

•    * For some vendor drivers, the hardware resources are shared between
  

•    * writeback encoder and rest of the display pipeline.
  

•    * To accommodate such cases, encoder is a handle to the real encoder
  

•    * hardware.
  

•    *
  

•    * For current existing writeback users, this shall continue to be the
  

•    * embedded encoder for the writeback connector.
  

•    */
  

•   struct drm_encoder *encoder;
  

•   /**
  

•    * @internal_encoder: internal encoder used by writeback when
  

•    * drm_writeback_connector_init() is used.
  

•    * @encoder will be assigned to this for those cases
  

•    *
  

•    * This will be unused when drm_writeback_connector_init_with_encoder()
  

•    * is used.
     */
  

•   struct drm_encoder encoder;
  

•   struct drm_encoder internal_encoder;
  
    /**
     * @pixel_formats_blob_ptr:
  

-- 2.7.4