[RFC,0/2] i915 writeback enablement

Message

Kandpal, Suraj June 1, 2022, 8:21 a.m. UTC

With this we try to enable writeback feature keeping with the drm_core
drm_writeback framework but to do this we have to create a drm_encoder and
drm_connector which is not present in intel_connector and intel_encoder
which causes all our iterators to bug out as they assume any drm_encoder
and drm_connector will naturally be embedded in the intel_ counterpart
structures but this cannot be acheived from drm_writeback_connector as
the drm_encoder and drm_connector fields in the structure are pointers
which does not allow us to embed them in our intel_ structures.I have 
tried to change some iterators and show what further changes may look
like but this will happen any and every place for_each_intel_encoder or
for_each_intel_connector_ is going to be used.I would like suggestions
on if moving forward with this approach would be useful or if the
private i915 framework mentioned below sounds more practical.
We previously tried to change the drm_connector and drm_encoder fields
into pointer in the drm_writeback_connector structure but faced a huge 
backlash from community.
Other than this approach we have another solution which won't be as much
of a work and will require minimal changes is where we take the whole
drm_writeback framework that is being used pull it into i915 making it
private and change the encoder and connector fields to pointers. The
approach has been floated in below series
[1] https://patchwork.freedesktop.org/series/103417/
Suraj Kandpal (2):
  drm/i915: Define WD trancoder for i915
  drm/i915: Enabling WD Transcoder

 drivers/gpu/drm/i915/Makefile                 |   1 +
 drivers/gpu/drm/i915/display/intel_acpi.c     |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  | 369 +++++++--
 drivers/gpu/drm/i915/display/intel_display.h  |  23 +
 .../drm/i915/display/intel_display_types.h    |  30 +
 drivers/gpu/drm/i915/display/intel_dpll.c     |   6 +
 drivers/gpu/drm/i915/display/intel_opregion.c |   3 +
 .../gpu/drm/i915/display/intel_wb_connector.h |  20 +
 drivers/gpu/drm/i915/display/intel_wd.c       | 748 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_wd.h       |  84 ++
 drivers/gpu/drm/i915/i915_drv.h               |   4 +
 drivers/gpu/drm/i915/i915_irq.c               |   8 +-
 drivers/gpu/drm/i915/i915_pci.c               |   7 +-
 drivers/gpu/drm/i915/i915_reg.h               | 139 ++++
 14 files changed, 1395 insertions(+), 48 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_wb_connector.h
 create mode 100644 drivers/gpu/drm/i915/display/intel_wd.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_wd.h

Comments

Kandpal, Suraj June 13, 2022, 5:15 a.m. UTC | #1

> With this we try to enable writeback feature keeping with the drm_core
> drm_writeback framework but to do this we have to create a drm_encoder
> and drm_connector which is not present in intel_connector and
> intel_encoder which causes all our iterators to bug out as they assume any
> drm_encoder and drm_connector will naturally be embedded in the intel_
> counterpart structures but this cannot be acheived from
> drm_writeback_connector as the drm_encoder and drm_connector fields in
> the structure are pointers which does not allow us to embed them in our
> intel_ structures.I have tried to change some iterators and show what further
> changes may look like but this will happen any and every place
> for_each_intel_encoder or for_each_intel_connector_ is going to be used.I
> would like suggestions on if moving forward with this approach would be
> useful or if the private i915 framework mentioned below sounds more
> practical.
> We previously tried to change the drm_connector and drm_encoder fields
> into pointer in the drm_writeback_connector structure but faced a huge
> backlash from community.
> Other than this approach we have another solution which won't be as much
> of a work and will require minimal changes is where we take the whole
> drm_writeback framework that is being used pull it into i915 making it
> private and change the encoder and connector fields to pointers. The
> approach has been floated in below series [1]
> https://patchwork.freedesktop.org/series/103417/


Hi Daniel,
Any suggestions how we can proceed with this.

BR,
Suraj Kandpal