Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On 10-10-18, 09:10, Jordan Crouse wrote: > On Wed, Oct 10, 2018 at 08:21:39PM +0530, Viresh Kumar wrote: > > On 10-10-18, 08:48, Jordan Crouse wrote: > > > qcom,level comes straight from: > > > > > > https://lore.kernel.org/lkml/20180627045234.27403-2-rna...@codeaurora.org/ > > > > > > But in this case instead of using the CPU to program the RPMh we are > > > passing > > > the value to a microprocessor (the GMU) and that will do the vote on our > > > behalf > > > (Technically we use the value to look up the vote in the cmd-db database > > > and > > > pass that to the GMU) > > > > > > This is why the qcom,level was added in the first place so we could at > > > least > > > share the nomenclature with the rpmhd if not the implementation. > > > > How you actually pass the vote to the underlying hardware, RPMh or > > GMU, is irrelevant to the whole thing. What is important is how we > > describe that in DT and how we represent the whole thing. > > > > We have chosen genpd + OPP to do this and same should be used by you > > as well. Another benefit is that the genpd core will do vote > > aggregation for you here. > > I'm not sure what you are suggesting? The vote is represented in DT exactly as > described in the bindings. Look at how Rajendra has done it to see the difference. https://lore.kernel.org/lkml/20180627045234.27403-3-rna...@codeaurora.org/ -- viresh ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v4] drm/msm: validate display and event threads
While creating display and event threads per crtc, validate
them before setting their priorities.
changes in v2:
- use dev_warn (Abhinav Kumar)
changes in v3:
- fix compilation error
changes in v4:
- Remove Change-Id (Sean Paul)
- Keep logging within 80 char limit (Sean Paul)
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/msm_drv.c | 49 ++-
1 file changed, 16 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 4904d0d..dcff812 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -553,17 +553,18 @@ static int msm_drm_init(struct device *dev, struct
drm_driver *drv)
kthread_run(kthread_worker_fn,
>disp_thread[i].worker,
"crtc_commit:%d", priv->disp_thread[i].crtc_id);
- ret = sched_setscheduler(priv->disp_thread[i].thread,
- SCHED_FIFO, );
- if (ret)
- pr_warn("display thread priority update failed: %d\n",
- ret);
-
if (IS_ERR(priv->disp_thread[i].thread)) {
dev_err(dev, "failed to create crtc_commit kthread\n");
priv->disp_thread[i].thread = NULL;
+ goto err_msm_uninit;
}
+ ret = sched_setscheduler(priv->disp_thread[i].thread,
+SCHED_FIFO, );
+ if (ret)
+ dev_warn(dev, "disp_thread set priority failed: %d\n",
+ret);
+
/* initialize event thread */
priv->event_thread[i].crtc_id = priv->crtcs[i]->base.id;
kthread_init_worker(>event_thread[i].worker);
@@ -572,6 +573,12 @@ static int msm_drm_init(struct device *dev, struct
drm_driver *drv)
kthread_run(kthread_worker_fn,
>event_thread[i].worker,
"crtc_event:%d", priv->event_thread[i].crtc_id);
+ if (IS_ERR(priv->event_thread[i].thread)) {
+ dev_err(dev, "failed to create crtc_event kthread\n");
+ priv->event_thread[i].thread = NULL;
+ goto err_msm_uninit;
+ }
+
/**
* event thread should also run at same priority as disp_thread
* because it is handling frame_done events. A lower priority
@@ -580,34 +587,10 @@ static int msm_drm_init(struct device *dev, struct
drm_driver *drv)
* failure at crtc commit level.
*/
ret = sched_setscheduler(priv->event_thread[i].thread,
- SCHED_FIFO, );
+SCHED_FIFO, );
if (ret)
- pr_warn("display event thread priority update failed:
%d\n",
- ret);
-
- if (IS_ERR(priv->event_thread[i].thread)) {
- dev_err(dev, "failed to create crtc_event kthread\n");
- priv->event_thread[i].thread = NULL;
- }
-
- if ((!priv->disp_thread[i].thread) ||
- !priv->event_thread[i].thread) {
- /* clean up previously created threads if any */
- for ( ; i >= 0; i--) {
- if (priv->disp_thread[i].thread) {
- kthread_stop(
- priv->disp_thread[i].thread);
- priv->disp_thread[i].thread = NULL;
- }
-
- if (priv->event_thread[i].thread) {
- kthread_stop(
- priv->event_thread[i].thread);
- priv->event_thread[i].thread = NULL;
- }
- }
- goto err_msm_uninit;
- }
+ dev_warn(dev, "event_thread set priority failed:%d\n",
+ret);
}
ret = drm_vblank_init(ddev, priv->num_crtcs);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 14/25] drm/msm/dpu: remove enc_id tagging for hw blocks
On 2018-10-10 08:06, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:31PM -0700, Jeykumar Sankaran wrote:
RM was using encoder id's to tag HW block's to reserve
and retrieve later for display pipeline. Now
that all the reserved HW blocks for a display are
maintained in its crtc state, no retrieval is needed.
This patch cleans up RM of encoder id tagging.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 90
+--
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 --
2 files changed, 36 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index 303f1b3..a8461b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -21,9 +21,6 @@
#include "dpu_encoder.h"
#include "dpu_trace.h"
-#define RESERVED_BY_OTHER(h, r) \
- ((h)->enc_id && (h)->enc_id != r)
-
/**
* struct dpu_rm_requirements - Reservation requirements parameter
bundle
* @topology: selected topology for the display
@@ -38,12 +35,13 @@ struct dpu_rm_requirements {
/**
* struct dpu_rm_hw_blk - hardware block tracking list member
* @list: List head for list of all hardware blocks tracking items
- * @enc_id:Encoder id to which this blk is binded
+ * @in_use: True, if the hw block is assigned to a display
pipeline.
+ * False, otherwise
* @hw:Pointer to the hardware register access object for
this block
*/
struct dpu_rm_hw_blk {
struct list_head list;
- uint32_t enc_id;
+ bool in_use;
How do the reservations work for TEST_ONLY commits? At a quick glance
it
looks
like they might be marked in_use?
Yes. We have a bug. I guess I should be releasing them in
drm_crtc_destroy_state.
Thanks and Regards,
Jeykumar S.
Sean
struct dpu_hw_blk *hw;
};
@@ -51,23 +49,19 @@ struct dpu_rm_hw_blk {
* struct dpu_rm_hw_iter - iterator for use with dpu_rm
* @hw: dpu_hw object requested, or NULL on failure
* @blk: dpu_rm internal block representation. Clients ignore. Used
as
iterator.
- * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for
Any
Encoder
* @type: Hardware Block Type client wishes to search for.
*/
struct dpu_rm_hw_iter {
struct dpu_hw_blk *hw;
struct dpu_rm_hw_blk *blk;
- uint32_t enc_id;
enum dpu_hw_blk_type type;
};
static void _dpu_rm_init_hw_iter(
struct dpu_rm_hw_iter *iter,
- uint32_t enc_id,
enum dpu_hw_blk_type type)
{
memset(iter, 0, sizeof(*iter));
- iter->enc_id = enc_id;
iter->type = type;
}
@@ -91,16 +85,12 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm
*rm,
struct dpu_rm_hw_iter *i)
i->blk = list_prepare_entry(i->blk, blk_list, list);
list_for_each_entry_continue(i->blk, blk_list, list) {
- if (i->enc_id == i->blk->enc_id) {
+ if (!i->blk->in_use) {
i->hw = i->blk->hw;
- DPU_DEBUG("found type %d id %d for enc %d\n",
- i->type, i->blk->hw->id,
i->enc_id);
return true;
}
}
- DPU_DEBUG("no match, type %d for enc %d\n", i->type, i->enc_id);
-
return false;
}
@@ -196,7 +186,6 @@ static int _dpu_rm_hw_blk_create(
}
blk->hw = hw;
- blk->enc_id = 0;
list_add_tail(>list, >hw_blks[type]);
return 0;
@@ -301,7 +290,6 @@ static bool _dpu_rm_needs_split_display(const
struct
msm_display_topology *top)
* proposed use case requirements, incl. hardwired dependent blocks
like
* pingpong
* @rm: dpu resource manager handle
- * @enc_id: encoder id requesting for allocation
* @reqs: proposed use case requirements
* @lm: proposed layer mixer, function checks if lm, and all other
hardwired
* blocks connected to the lm (pp) is available and appropriate
@@ -313,7 +301,6 @@ static bool _dpu_rm_needs_split_display(const
struct
msm_display_topology *top)
*/
static bool _dpu_rm_check_lm_and_get_connected_blks(
struct dpu_rm *rm,
- uint32_t enc_id,
struct dpu_rm_requirements *reqs,
struct dpu_rm_hw_blk *lm,
struct dpu_rm_hw_blk **pp,
@@ -339,13 +326,7 @@ static bool
_dpu_rm_check_lm_and_get_connected_blks(
}
}
- /* Already reserved? */
- if (RESERVED_BY_OTHER(lm, enc_id)) {
- DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
- return false;
- }
-
- _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
+ _dpu_rm_init_hw_iter(, DPU_HW_BLK_PINGPONG);
while (_dpu_rm_get_hw_locked(rm, )) {
if (iter.blk->hw->id == lm_cfg->pingpong) {
*pp = iter.blk;
@@ -358,16 +339,10 @@
Re: [Freedreno] [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support
On Wed, Oct 10, 2018 at 10:15:58AM -0700, Chandan Uddaraju wrote: > Add the needed displayPort files to enable DP driver > on msm target. > > "dp_display" module is the main module that calls into > other sub-modules. "dp_drm" file represents the interface > between DRM framework and DP driver. > > Signed-off-by: Chandan Uddaraju > --- > drivers/gpu/drm/msm/Kconfig |9 + > drivers/gpu/drm/msm/Makefile| 15 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c | 206 > drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h | 44 + > drivers/gpu/drm/msm/dp/dp_aux.c | 570 ++ > drivers/gpu/drm/msm/dp/dp_aux.h | 44 + > drivers/gpu/drm/msm/dp/dp_catalog.c | 1188 > drivers/gpu/drm/msm/dp/dp_catalog.h | 144 +++ > drivers/gpu/drm/msm/dp/dp_ctrl.c| 1475 + > drivers/gpu/drm/msm/dp/dp_ctrl.h| 50 + > drivers/gpu/drm/msm/dp/dp_debug.c | 507 + > drivers/gpu/drm/msm/dp/dp_debug.h | 81 ++ > drivers/gpu/drm/msm/dp/dp_display.c | 977 + > drivers/gpu/drm/msm/dp/dp_display.h | 55 + > drivers/gpu/drm/msm/dp/dp_drm.c | 542 ++ > drivers/gpu/drm/msm/dp/dp_drm.h | 52 + > drivers/gpu/drm/msm/dp/dp_extcon.c | 400 +++ > drivers/gpu/drm/msm/dp/dp_extcon.h | 111 ++ > drivers/gpu/drm/msm/dp/dp_link.c| 1549 > +++ > drivers/gpu/drm/msm/dp/dp_link.h| 184 > drivers/gpu/drm/msm/dp/dp_panel.c | 624 +++ > drivers/gpu/drm/msm/dp/dp_panel.h | 121 +++ > drivers/gpu/drm/msm/dp/dp_parser.c | 679 > drivers/gpu/drm/msm/dp/dp_parser.h | 205 > drivers/gpu/drm/msm/dp/dp_power.c | 599 +++ > drivers/gpu/drm/msm/dp/dp_power.h | 57 + > drivers/gpu/drm/msm/dp/dp_reg.h | 357 ++ > drivers/gpu/drm/msm/msm_drv.c |2 + > drivers/gpu/drm/msm/msm_drv.h | 22 + > include/drm/drm_dp_helper.h | 19 + > 30 files changed, 10887 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_extcon.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_link.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_link.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_panel.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_parser.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_power.c > create mode 100644 drivers/gpu/drm/msm/dp/dp_power.h > create mode 100644 drivers/gpu/drm/msm/dp/dp_reg.h > > diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig > index 843a9d4..c363f24 100644 > --- a/drivers/gpu/drm/msm/Kconfig > +++ b/drivers/gpu/drm/msm/Kconfig > @@ -49,6 +49,15 @@ config DRM_MSM_HDMI_HDCP > help > Choose this option to enable HDCP state machine > > +config DRM_MSM_DP > + bool "Enable DP support in MSM DRM driver" > + depends on DRM_MSM > + default n default n should be implied so you don't need to add it. > + help > + Compile in support for DP driver in msm drm > + driver. DP external display support is enabled > + through this config option. It can be primary or > + secondary display on device. > config DRM_MSM_DSI > bool "Enable DSI support in MSM DRM driver" > depends on DRM_MSM > diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile > index 19ab521..765a8d8 100644 > --- a/drivers/gpu/drm/msm/Makefile > +++ b/drivers/gpu/drm/msm/Makefile > @@ -2,6 +2,7 @@ > ccflags-y := -Idrivers/gpu/drm/msm > ccflags-y += -Idrivers/gpu/drm/msm/disp/dpu1 > ccflags-$(CONFIG_DRM_MSM_DSI) += -Idrivers/gpu/drm/msm/dsi > +ccflags-$(CONFIG_DRM_MSM_DP) += -Idrivers/gpu/drm/msm/dp > > msm-y := \ > adreno/adreno_device.o \ > @@ -93,7 +94,19 @@ msm-y := \ > msm_submitqueue.o > > msm-$(CONFIG_DEBUG_FS) += adreno/a5xx_debugfs.o \ > - disp/dpu1/dpu_dbg.o > +
Re: [Freedreno] [PATCH 24/25] drm/msm/dpu: remove mutex locking for RM interfaces
On 2018-10-10 07:36, Sean Paul wrote:
On Tue, Oct 09, 2018 at 11:03:24PM -0700, Jeykumar Sankaran wrote:
On 2018-10-09 12:57, Sean Paul wrote:
> On Mon, Oct 08, 2018 at 09:27:41PM -0700, Jeykumar Sankaran wrote:
> > Since HW reservations are happening through atomic_check
> > and all the display commits are catered by a single commit thread,
> > it is not necessary to protect the interfaces by a separate
> > mutex.
> >
> > Signed-off-by: Jeykumar Sankaran
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 24
> > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 --
> > 2 files changed, 26 deletions(-)
> >
>
> /snip
>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > index 8676fa5..9acbeba 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > @@ -24,11 +24,9 @@
> > * struct dpu_rm - DPU dynamic hardware resource manager
> > * @hw_blks: array of lists of hardware resources present in the
> system, one
> > * list per type of hardware block
> > - * @rm_lock: resource manager mutex
> > */
> > struct dpu_rm {
> > struct list_head hw_blks[DPU_HW_BLK_MAX];
>
> At this point, there's really not much point to even having the rm.
It's
> just
> another level of indirection that IMO complicates the code. If you
look
> at the usage of hw_blks, the code is always looking at a specific type
> of
> hw_blk, so the array is unnecessary.
>
> dpu_kms could just keep a few arrays/lists of the hw types, and the
crtc
> and encoder
> reserve functions can just go in crtc/encoder.
>
> Sean
>
RM has been reduced to its current form to manage only LM/PP, CTL and
interfaces.
Our eventual plan is to support all the advanced HW blocks and its
features
in
an upstream friendly way. When RM grows to manage all its subblocks,
iteration
logic may get heavy since the chipset have HW chain restrictions on
various
hw blocks.
To provide room for the growth, I suggest keeping the allocation
helpers in a separate file. But I can see why you want to maintain the
HW
block lists
in the KMS.
At least for the blocks that exist, using the RM is unnecessary, does
that
change for the current blocks when you add more? I'm guessing their
code
will
remain unchanged.
Yes. But to seperate out the allocation logics, I prefered the separate
file. I guess we can hold off the discussion until we need those
enhancements.
I can get rid of the RM files for now and move the allocation functions
to
the respective files (CRTC / Encoder).
Thanks,
Jeykumar S.
If the new blocks you're adding have a lot of commonality, perhaps it
makes
sense to re-introduce the RM, but IMO it doesn't make sense for
lm/ctl/pp.
Sean
Thanks,
Jeykumar S.
> > - struct mutex rm_lock;
> > };
> >
> > /**
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> > a Linux Foundation Collaborative Project
> >
> > ___
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
--
Jeykumar S
--
Jeykumar S
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 01/25] drm/msm/dpu: fix hw ctl retrieval for mixer muxing
On 2018-10-10 07:29, Sean Paul wrote:
On Tue, Oct 09, 2018 at 10:46:41PM -0700, Jeykumar Sankaran wrote:
On 2018-10-09 11:07, Sean Paul wrote:
> On Mon, Oct 08, 2018 at 09:27:18PM -0700, Jeykumar Sankaran wrote:
> > Layer mixer/pingpong block counts and hw ctl block counts
> > will not be same for all the topologies (e.g. layer
> > mixer muxing to single interface)
> >
> > Use the encoder's split_role info to retrieve the
> > respective control path for programming.
> >
> > Signed-off-by: Jeykumar Sankaran
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 96cdf06..d12f896 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -1060,6 +1060,7 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> >
> > for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> > + int ctl_index;
> >
> > if (phys) {
> > if (!dpu_enc->hw_pp[i]) {
> > @@ -1068,14 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> > return;
> > }
> >
> > - if (!hw_ctl[i]) {
> > + ctl_index = phys->split_role == ENC_ROLE_SLAVE ? 1
> : 0;
> > +
>
> What if MAX_CHANNELS_PER_ENC isn't 2? Similarly, what if num_phys_encs
> MAX_CHANNELS_PER_ENC? It seems like there should be a more formal
> relationship
> between all of these verious values (num_of_h_tiles assumed to be <= 2
> as
> well).
> If one of them changes beyond the assumed bound, the rest of the
driver
> falls
> over pretty hard.
>
MAX_CHANNELS_PER_ENC is set to 2 to represent HW limitation on the
chipset
as
we cannot gang up more than 2 LM chain to an interface. Supporting
more
than
2
might demand much larger changes than validating for boundaries.
num_phys_enc is the max no of phys encoders we create as we are
looping
through
num_of_h_tiles which cannot be more than priv->dsi array size.
So its very unlikely we would expect these loops to go out of bound!
For now, sure. However a new revision of hardware will be a pain to add
support
for if we add more assumptions, and secondly it makes it _really_ hard
to
understand the code if you don't have Qualcomm employee-level access to
the
hardware design :).
I am having a hard time understanding why you have to see these counts
as
"assumptions".
Except for MAX_CHANNELS_PER_ENC, all the other counts are either
calculated
or derived from the other modules linked to the topology.
h_tiles is the drm_connector terminology which represents the number of
panels
the display is driving. We use this information to determine the HW
block chains in the MDP. HW blocks counts (pp or ctl) need not be same
as the h_tile count to replace them with.
I believe maintaining the counts independently at each layer allows us
to have more
flexibility to support independent HW chaining for future revisions.
Would it be more convincing if I get the MAX_CHANNELS_PER_ENC value from
catalog.c?
So this is why I'm advocating for the reduction of the number of
"num_of_"
values we assume are all in the same range. It's a lot easier to
understand the
hardware when you can see that a phys encoder is needed per h tile, and
that a
ctl/pp is needed per phys encoder.
This is exactly the idea I don't want to convey to the reader. For the
LM merge path,
each phys encoder will not be having its own control. Based on the
topology we
are supporting, HW block counts can vary. We can even drive:
- 2 interfaces with 1 ctl and 1 ping pong
- 1 interface with 1 ctl and 2 ping pongs
- 1 interface with 1 ctl and 1 ping pong
Thanks,
Jeykumar S.
Anyways, just my $0.02.
Sean
Thanks,
Jeykumar S.
>
> > + if (!hw_ctl[ctl_index]) {
> > DPU_ERROR_ENC(dpu_enc, "no ctl block
> assigned"
> > - "at idx: %d\n", i);
> > + "at idx: %d\n", ctl_index);
> > return;
>
> When you return on error here, should you give back the resources that
> you've
> already provisioned?
>
> > }
> >
> > phys->hw_pp = dpu_enc->hw_pp[i];
> > - phys->hw_ctl = hw_ctl[i];
> > + phys->hw_ctl = hw_ctl[ctl_index];
> >
> > phys->connector = conn->state->connector;
> > if (phys->ops.mode_set)
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
> > a Linux Foundation Collaborative Project
> >
> > ___
> > Freedreno
Re: [Freedreno] [DPU PATCH 0/3] Add support for DisplayPort driver on SnapDragon 845
On 2018-10-10 10:15, Chandan Uddaraju wrote: These patches add support for Display-Port driver on SnapDragon 845 hardware. It adds DP driver and DP PLL driver files along with the needed device-tree bindings. The block diagram of DP driver is shown below: +-+ |DRM FRAMEWORK| +--+--+ | +v+ | DP DRM | +++ | +v+ ++| DP+--++--+ ++---+| DISPLAY |+---+ | | |++-+-+-+| | | || | | | | | || | | | | | || | | | | | vv v v v v v +--+ +--+ +---+ ++ ++ +---+ +-+ | DP | | DP | |DP | | DP | | DP | |DP | | DP | |PARSER| |EXTCON| |AUX| |LINK| |CTRL| |PHY| |POWER| +--+---+ +---+--+ +---+ ++ +--+-+ +-+-+ +-+ | || | +--v---+ +---v-+ +v-v+ |DEVICE| |EXTCON | | DP | | TREE | |INTERFACE| |CATALOG| +--+ +-+ +---+---+ | +---v+ |CTRL/PHY| | HW | ++ It will be more helpful to have this block diagram in [DPU PATCH 2/3] drm/msm/dp: add displayPort driver support Thanks, Jeykumar S. These patches have dependency on clock driver changes mentioned below: https://patchwork.kernel.org/patch/10632753/ https://patchwork.kernel.org/patch/10632757/ Chandan Uddaraju (3): dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon 845 drm/msm/dp: add displayPort driver support drm/msm/dp: add support for DP PLL driver .../devicetree/bindings/display/msm/dp.txt | 249 .../devicetree/bindings/display/msm/dpu.txt| 16 +- drivers/gpu/drm/msm/Kconfig| 25 + drivers/gpu/drm/msm/Makefile | 21 +- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c| 206 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h| 44 + drivers/gpu/drm/msm/dp/dp_aux.c| 570 +++ drivers/gpu/drm/msm/dp/dp_aux.h| 44 + drivers/gpu/drm/msm/dp/dp_catalog.c| 1188 +++ drivers/gpu/drm/msm/dp/dp_catalog.h| 144 ++ drivers/gpu/drm/msm/dp/dp_ctrl.c | 1476 +++ drivers/gpu/drm/msm/dp/dp_ctrl.h | 50 + drivers/gpu/drm/msm/dp/dp_debug.c | 507 +++ drivers/gpu/drm/msm/dp/dp_debug.h | 81 + drivers/gpu/drm/msm/dp/dp_display.c| 1027 + drivers/gpu/drm/msm/dp/dp_display.h| 58 + drivers/gpu/drm/msm/dp/dp_drm.c| 542 +++ drivers/gpu/drm/msm/dp/dp_drm.h| 52 + drivers/gpu/drm/msm/dp/dp_extcon.c | 400 + drivers/gpu/drm/msm/dp/dp_extcon.h | 111 ++ drivers/gpu/drm/msm/dp/dp_link.c | 1549 drivers/gpu/drm/msm/dp/dp_link.h | 184 +++ drivers/gpu/drm/msm/dp/dp_panel.c | 624 drivers/gpu/drm/msm/dp/dp_panel.h | 121 ++ drivers/gpu/drm/msm/dp/dp_parser.c | 679 + drivers/gpu/drm/msm/dp/dp_parser.h | 208 +++ drivers/gpu/drm/msm/dp/dp_power.c | 652 drivers/gpu/drm/msm/dp/dp_power.h | 59 + drivers/gpu/drm/msm/dp/dp_reg.h| 357 + drivers/gpu/drm/msm/dp/pll/dp_pll.c| 153 ++ drivers/gpu/drm/msm/dp/pll/dp_pll.h| 64 + drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 + drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 ++ drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 +++ drivers/gpu/drm/msm/msm_drv.c |2 + drivers/gpu/drm/msm/msm_drv.h | 22 + include/drm/drm_dp_helper.h| 19 + 37 files changed, 12525 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.h create
Re: [Freedreno] [DPU PATCH 1/3] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon 845
On Wed, Oct 10, 2018 at 10:15:57AM -0700, Chandan Uddaraju wrote: > Add bindings for Snapdragon 845 DisplayPort and > display-port PLL driver. > This won't get Rob Herring's review unless it's sent to the devicetree list. > Signed-off-by: Chandan Uddaraju > --- > .../devicetree/bindings/display/msm/dp.txt | 249 > + > .../devicetree/bindings/display/msm/dpu.txt| 16 +- > 2 files changed, 261 insertions(+), 4 deletions(-) > create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt > > diff --git a/Documentation/devicetree/bindings/display/msm/dp.txt > b/Documentation/devicetree/bindings/display/msm/dp.txt > new file mode 100644 > index 000..0155266 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/dp.txt > @@ -0,0 +1,249 @@ > +Qualcomm Technologies, Inc. > +DP is the master Display Port device which supports DP host controllers that > are compatible with VESA Display Port interface specification. > +DP Controller: Required properties: > +- compatible: Should be "qcom,dp-display". > +- reg: Base address and length of DP hardware's memory > mapped regions. > +- cell-index: Specifies the controller instance. > +- reg-names:A list of strings that name the list of regs. > + "dp_ahb" - DP controller memory region. > + "dp_aux" - DP AUX memory region. > + "dp_link" - DP link layer memory region. > + "dp_p0" - DP pixel clock domain memory region. > + "dp_phy" - DP PHY memory region. > + "dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region. > + "dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region. > + "dp_mmss_cc" - Display Clock Control memory region. > + "qfprom_physical" - QFPROM Phys memory region. > + "dp_pll" - USB3 DP combo PLL memory region. > + "usb3_dp_com" - USB3 DP PHY combo memory region. > + "hdcp_physical" - DP HDCP memory region. > +- interrupt-parent phandle to the interrupt parent device node. > +- interrupts:The interrupt signal from the DP block. > +- clocks: Clocks required for Display Port operation. See [1] > for details on clock bindings. > +- clock-names: Names of the clocks corresponding to handles. > Following clocks are required: > + "core_aux_clk", > "core_usb_ref_clk_src","core_usb_ref_clk", "core_usb_cfg_ahb_clk", > + "core_usb_pipe_clk", "ctrl_link_clk", > "ctrl_link_iface_clk", "ctrl_crypto_clk", > + "ctrl_pixel_clk", "pixel_clk_rcg", "pixel_parent". > +- pll-node: phandle to DP PLL node. > +- vdda-1p2-supply: phandle to vdda 1.2V regulator node. > +- vdda-0p9-supply: phandle to vdda 0.9V regulator node. > +- qcom,aux-cfg0-settings:Specifies the DP AUX configuration 0 > settings. The first > + entry in this array corresponds to the > register offset > + within DP AUX, while the remaining > entries indicate the > + programmable values. > +- qcom,aux-cfg1-settings:Specifies the DP AUX configuration 1 > settings. The first > + entry in this array corresponds to the > register offset > + within DP AUX, while the remaining > entries indicate the > + programmable values. > +- qcom,aux-cfg2-settings:Specifies the DP AUX configuration 2 > settings. The first > + entry in this array corresponds to the > register offset > + within DP AUX, while the remaining > entries indicate the > + programmable values. > +- qcom,aux-cfg3-settings:Specifies the DP AUX configuration 3 > settings. The first > + entry in this array corresponds to the > register offset > + within DP AUX, while the remaining > entries indicate the > + programmable values. > +- qcom,aux-cfg4-settings:Specifies the DP AUX configuration 4 > settings. The first > + entry in this array corresponds to the > register offset > + within DP AUX, while the remaining > entries indicate the > + programmable values. > +- qcom,aux-cfg5-settings:Specifies the DP AUX configuration 5 > settings. The first > + entry in this array corresponds to the > register offset > +
[Freedreno] [DPU PATCH 0/3] Add support for DisplayPort driver on SnapDragon 845
These patches add support for Display-Port driver on SnapDragon 845 hardware. It adds DP driver and DP PLL driver files along with the needed device-tree bindings. The block diagram of DP driver is shown below: +-+ |DRM FRAMEWORK| +--+--+ | +v+ | DP DRM | +++ | +v+ ++| DP+--++--+ ++---+| DISPLAY |+---+ | | |++-+-+-+| | | || | | | | | || | | | | | || | | | | | vv v v v v v +--+ +--+ +---+ ++ ++ +---+ +-+ | DP | | DP | |DP | | DP | | DP | |DP | | DP | |PARSER| |EXTCON| |AUX| |LINK| |CTRL| |PHY| |POWER| +--+---+ +---+--+ +---+ ++ +--+-+ +-+-+ +-+ | || | +--v---+ +---v-+ +v-v+ |DEVICE| |EXTCON | | DP | | TREE | |INTERFACE| |CATALOG| +--+ +-+ +---+---+ | +---v+ |CTRL/PHY| | HW | ++ These patches have dependency on clock driver changes mentioned below: https://patchwork.kernel.org/patch/10632753/ https://patchwork.kernel.org/patch/10632757/ Chandan Uddaraju (3): dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon 845 drm/msm/dp: add displayPort driver support drm/msm/dp: add support for DP PLL driver .../devicetree/bindings/display/msm/dp.txt | 249 .../devicetree/bindings/display/msm/dpu.txt| 16 +- drivers/gpu/drm/msm/Kconfig| 25 + drivers/gpu/drm/msm/Makefile | 21 +- drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.c| 206 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_io_util.h| 44 + drivers/gpu/drm/msm/dp/dp_aux.c| 570 +++ drivers/gpu/drm/msm/dp/dp_aux.h| 44 + drivers/gpu/drm/msm/dp/dp_catalog.c| 1188 +++ drivers/gpu/drm/msm/dp/dp_catalog.h| 144 ++ drivers/gpu/drm/msm/dp/dp_ctrl.c | 1476 +++ drivers/gpu/drm/msm/dp/dp_ctrl.h | 50 + drivers/gpu/drm/msm/dp/dp_debug.c | 507 +++ drivers/gpu/drm/msm/dp/dp_debug.h | 81 + drivers/gpu/drm/msm/dp/dp_display.c| 1027 + drivers/gpu/drm/msm/dp/dp_display.h| 58 + drivers/gpu/drm/msm/dp/dp_drm.c| 542 +++ drivers/gpu/drm/msm/dp/dp_drm.h| 52 + drivers/gpu/drm/msm/dp/dp_extcon.c | 400 + drivers/gpu/drm/msm/dp/dp_extcon.h | 111 ++ drivers/gpu/drm/msm/dp/dp_link.c | 1549 drivers/gpu/drm/msm/dp/dp_link.h | 184 +++ drivers/gpu/drm/msm/dp/dp_panel.c | 624 drivers/gpu/drm/msm/dp/dp_panel.h | 121 ++ drivers/gpu/drm/msm/dp/dp_parser.c | 679 + drivers/gpu/drm/msm/dp/dp_parser.h | 208 +++ drivers/gpu/drm/msm/dp/dp_power.c | 652 drivers/gpu/drm/msm/dp/dp_power.h | 59 + drivers/gpu/drm/msm/dp/dp_reg.h| 357 + drivers/gpu/drm/msm/dp/pll/dp_pll.c| 153 ++ drivers/gpu/drm/msm/dp/pll/dp_pll.h| 64 + drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 + drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 ++ drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 +++ drivers/gpu/drm/msm/msm_drv.c |2 + drivers/gpu/drm/msm/msm_drv.h | 22 + include/drm/drm_dp_helper.h| 19 + 37 files changed, 12525 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.c create mode 100644 drivers/gpu/drm/msm/dp/dp_aux.h create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.c create mode 100644 drivers/gpu/drm/msm/dp/dp_catalog.h create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.c create mode 100644 drivers/gpu/drm/msm/dp/dp_ctrl.h create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.c create mode 100644 drivers/gpu/drm/msm/dp/dp_debug.h create mode 100644 drivers/gpu/drm/msm/dp/dp_display.c create mode 100644 drivers/gpu/drm/msm/dp/dp_display.h create mode 100644 drivers/gpu/drm/msm/dp/dp_drm.c create mode 100644
[Freedreno] [DPU PATCH 3/3] drm/msm/dp: add support for DP PLL driver
Add the needed DP PLL specific files to support
display port interface on msm targets.
The DP driver calls the DP PLL driver registration.
The DP driver sets the link and pixel clock sources.
Signed-off-by: Chandan Uddaraju
---
drivers/gpu/drm/msm/Kconfig | 16 +
drivers/gpu/drm/msm/Makefile | 6 +
drivers/gpu/drm/msm/dp/dp_ctrl.c | 1 +
drivers/gpu/drm/msm/dp/dp_display.c | 50 +++
drivers/gpu/drm/msm/dp/dp_display.h | 3 +
drivers/gpu/drm/msm/dp/dp_parser.h| 3 +
drivers/gpu/drm/msm/dp/dp_power.c | 77 +++-
drivers/gpu/drm/msm/dp/dp_power.h | 2 +
drivers/gpu/drm/msm/dp/pll/dp_pll.c | 153
drivers/gpu/drm/msm/dp/pll/dp_pll.h | 64
drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c | 401 +++
drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h | 94 +
drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c | 531 ++
13 files changed, 1389 insertions(+), 12 deletions(-)
create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.c
create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll.h
create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.c
create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm.h
create mode 100644 drivers/gpu/drm/msm/dp/pll/dp_pll_10nm_util.c
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index c363f24..1e0b9158 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -58,6 +58,22 @@ config DRM_MSM_DP
driver. DP external display support is enabled
through this config option. It can be primary or
secondary display on device.
+
+config DRM_MSM_DP_PLL
+ bool "Enable DP PLL driver in MSM DRM"
+ depends on DRM_MSM_DP && COMMON_CLK
+ default y
+ help
+ Choose this option to enable DP PLL driver which provides DP
+ source clocks under common clock framework.
+
+config DRM_MSM_DP_10NM_PLL
+ bool "Enable DP 10nm PLL driver in MSM DRM (used by SDM845)"
+ depends on DRM_MSM_DP
+ default y
+ help
+ Choose this option if DP PLL on SDM845 is used on the platform.
+
config DRM_MSM_DSI
bool "Enable DSI support in MSM DRM driver"
depends on DRM_MSM
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 765a8d8..8d18353 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -137,4 +137,10 @@ msm-$(CONFIG_DRM_MSM_DSI_14NM_PHY) +=
dsi/pll/dsi_pll_14nm.o
msm-$(CONFIG_DRM_MSM_DSI_10NM_PHY) += dsi/pll/dsi_pll_10nm.o
endif
+ifeq ($(CONFIG_DRM_MSM_DP_PLL),y)
+msm-y += dp/pll/dp_pll.o
+msm-y += dp/pll/dp_pll_10nm.o
+msm-y += dp/pll/dp_pll_10nm_util.o
+endif
+
obj-$(CONFIG_DRM_MSM) += msm.o
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 08a52f5..e23beee 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -1051,6 +1051,7 @@ static int dp_ctrl_enable_mainlink_clocks(struct
dp_ctrl_private *ctrl)
{
int ret = 0;
+ ctrl->power->set_link_clk_parent(ctrl->power);
ctrl->power->set_pixel_clk_parent(ctrl->power);
dp_ctrl_set_clock_rate(ctrl, "ctrl_link_clk",
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 8c98399..2bf6635 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -72,6 +72,48 @@ struct dp_display_private {
{}
};
+static int dp_get_pll(struct dp_display_private *dp_priv)
+{
+ struct platform_device *pdev = NULL;
+ struct platform_device *pll_pdev;
+ struct device_node *pll_node;
+ struct dp_parser *dp_parser = NULL;
+
+ if (!dp_priv) {
+ pr_err("Invalid Arguments\n");
+ return -EINVAL;
+ }
+
+ pdev = dp_priv->pdev;
+ dp_parser = dp_priv->parser;
+
+ if (!dp_parser) {
+ pr_err("Parser not initialized.\n");
+ return -EINVAL;
+ }
+
+ pll_node = of_parse_phandle(pdev->dev.of_node, "pll-node", 0);
+ if (!pll_node) {
+ dev_err(>dev, "cannot find pll device\n");
+ return -ENXIO;
+ }
+
+ pll_pdev = of_find_device_by_node(pll_node);
+ if (pll_pdev)
+ dp_parser->pll = platform_get_drvdata(pll_pdev);
+
+ of_node_put(pll_node);
+
+ if (!pll_pdev || !dp_parser->pll) {
+ dev_err(>dev, "%s: pll driver is not ready\n", __func__);
+ return -EPROBE_DEFER;
+ }
+
+ dp_parser->pll_dev = get_device(_pdev->dev);
+
+ return 0;
+}
+
static irqreturn_t dp_display_irq(int irq, void *dev_id)
{
struct dp_display_private *dp = dev_id;
@@ -125,6 +167,12 @@ static int dp_display_bind(struct device *dev, struct
device *master,
goto end;
}
+ rc =
[Freedreno] [DPU PATCH 1/3] dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon 845
Add bindings for Snapdragon 845 DisplayPort and display-port PLL driver. Signed-off-by: Chandan Uddaraju --- .../devicetree/bindings/display/msm/dp.txt | 249 + .../devicetree/bindings/display/msm/dpu.txt| 16 +- 2 files changed, 261 insertions(+), 4 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/dp.txt diff --git a/Documentation/devicetree/bindings/display/msm/dp.txt b/Documentation/devicetree/bindings/display/msm/dp.txt new file mode 100644 index 000..0155266 --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/dp.txt @@ -0,0 +1,249 @@ +Qualcomm Technologies, Inc. +DP is the master Display Port device which supports DP host controllers that are compatible with VESA Display Port interface specification. +DP Controller: Required properties: +- compatible: Should be "qcom,dp-display". +- reg: Base address and length of DP hardware's memory mapped regions. +- cell-index: Specifies the controller instance. +- reg-names:A list of strings that name the list of regs. + "dp_ahb" - DP controller memory region. + "dp_aux" - DP AUX memory region. + "dp_link" - DP link layer memory region. + "dp_p0" - DP pixel clock domain memory region. + "dp_phy" - DP PHY memory region. + "dp_ln_tx0" - USB3 DP PHY combo TX-0 lane memory region. + "dp_ln_tx1" - USB3 DP PHY combo TX-1 lane memory region. + "dp_mmss_cc" - Display Clock Control memory region. + "qfprom_physical" - QFPROM Phys memory region. + "dp_pll" - USB3 DP combo PLL memory region. + "usb3_dp_com" - USB3 DP PHY combo memory region. + "hdcp_physical" - DP HDCP memory region. +- interrupt-parent phandle to the interrupt parent device node. +- interrupts: The interrupt signal from the DP block. +- clocks: Clocks required for Display Port operation. See [1] for details on clock bindings. +- clock-names: Names of the clocks corresponding to handles. Following clocks are required: + "core_aux_clk", "core_usb_ref_clk_src","core_usb_ref_clk", "core_usb_cfg_ahb_clk", + "core_usb_pipe_clk", "ctrl_link_clk", "ctrl_link_iface_clk", "ctrl_crypto_clk", + "ctrl_pixel_clk", "pixel_clk_rcg", "pixel_parent". +- pll-node:phandle to DP PLL node. +- vdda-1p2-supply: phandle to vdda 1.2V regulator node. +- vdda-0p9-supply: phandle to vdda 0.9V regulator node. +- qcom,aux-cfg0-settings: Specifies the DP AUX configuration 0 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg1-settings: Specifies the DP AUX configuration 1 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg2-settings: Specifies the DP AUX configuration 2 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg3-settings: Specifies the DP AUX configuration 3 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg4-settings: Specifies the DP AUX configuration 4 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg5-settings: Specifies the DP AUX configuration 5 settings. The first + entry in this array corresponds to the register offset + within DP AUX, while the remaining entries indicate the + programmable values. +- qcom,aux-cfg6-settings: Specifies the DP AUX configuration 6 settings. The first +
Re: [Freedreno] [PATCH 16/25] drm/msm/dpu: clean up test_only flag for RM reservation
On Mon, Oct 08, 2018 at 09:27:33PM -0700, Jeykumar Sankaran wrote:
> Encoder uses test_only flag to differentiate RM reservations
> invoked from atomic check and atomic_commit phases.
> After reserving the HW blocks, if test_only was set, RM
> releases the reservation. Retains them if not. Since we
> got rid of RM reserve call from atomic_commit path, get rid
> of this flag.
I think I'm being dense, but I still don't see how the test_only path doesn't
result in lasting reservations.
Sean
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 13 +++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 4 +---
> 3 files changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 468b8fd0..dd17528 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -636,7 +636,7 @@ static int dpu_encoder_virt_atomic_check(
>
> if (!ret && drm_atomic_crtc_needs_modeset(crtc_state))
> ret = dpu_rm_reserve(_kms->rm, drm_enc, crtc_state,
> - topology, false);
> + topology);
>
> if (!ret)
> drm_mode_set_crtcinfo(adj_mode, 0);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 3a92a3e..1234991 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -631,8 +631,7 @@ int dpu_rm_reserve(
> struct dpu_rm *rm,
> struct drm_encoder *enc,
> struct drm_crtc_state *crtc_state,
> - struct msm_display_topology topology,
> - bool test_only)
> + struct msm_display_topology topology)
> {
> struct dpu_rm_requirements reqs;
> struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
> @@ -642,8 +641,8 @@ int dpu_rm_reserve(
> if (!drm_atomic_crtc_needs_modeset(crtc_state))
> return 0;
>
> - DRM_DEBUG_KMS("reserving hw for enc %d crtc %d test_only %d\n",
> - enc->base.id, crtc_state->crtc->base.id, test_only);
> + DRM_DEBUG_KMS("reserving hw for enc %d crtc %d\n",
> + enc->base.id, crtc_state->crtc->base.id);
>
> mutex_lock(>rm_lock);
>
> @@ -657,13 +656,7 @@ int dpu_rm_reserve(
> if (ret) {
> DPU_ERROR("failed to reserve hw resources: %d\n", ret);
> _dpu_rm_release_reservation(rm, dpu_cstate);
> - } else if (test_only) {
> - /* test_only: test the reservation and then undo */
> - DPU_DEBUG("test_only: discard test [enc: %d]\n",
> - enc->base.id);
> - _dpu_rm_release_reservation(rm, dpu_cstate);
> }
> -
> end:
> mutex_unlock(>rm_lock);
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index 7ac1553..415eeec 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -63,14 +63,12 @@ int dpu_rm_init(struct dpu_rm *rm,
> * @drm_enc: DRM Encoder handle
> * @crtc_state: Proposed Atomic DRM CRTC State handle
> * @topology: Pointer to topology info for the display
> - * @test_only: Atomic-Test phase, discard results (unless property overrides)
> * @Return: 0 on Success otherwise -ERROR
> */
> int dpu_rm_reserve(struct dpu_rm *rm,
> struct drm_encoder *drm_enc,
> struct drm_crtc_state *crtc_state,
> - struct msm_display_topology topology,
> - bool test_only);
> + struct msm_display_topology topology);
>
> /**
> * dpu_rm_release - Given the encoder for the display chain, release any
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On Wed, Oct 10, 2018 at 08:21:39PM +0530, Viresh Kumar wrote: > On 10-10-18, 08:48, Jordan Crouse wrote: > > qcom,level comes straight from: > > > > https://lore.kernel.org/lkml/20180627045234.27403-2-rna...@codeaurora.org/ > > > > But in this case instead of using the CPU to program the RPMh we are passing > > the value to a microprocessor (the GMU) and that will do the vote on our > > behalf > > (Technically we use the value to look up the vote in the cmd-db database and > > pass that to the GMU) > > > > This is why the qcom,level was added in the first place so we could at least > > share the nomenclature with the rpmhd if not the implementation. > > How you actually pass the vote to the underlying hardware, RPMh or > GMU, is irrelevant to the whole thing. What is important is how we > describe that in DT and how we represent the whole thing. > > We have chosen genpd + OPP to do this and same should be used by you > as well. Another benefit is that the genpd core will do vote > aggregation for you here. I'm not sure what you are suggesting? The vote is represented in DT exactly as described in the bindings. Jordan > -- > viresh > ___ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 15/25] drm/msm/dpu: avoid redundant hw blk reference
On Mon, Oct 08, 2018 at 09:27:32PM -0700, Jeykumar Sankaran wrote:
> Get rid of hw block pointer in RM iter as we can
> access the same through dpu_hw_blk.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 10 ++
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index a8461b8..3a92a3e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -47,12 +47,10 @@ struct dpu_rm_hw_blk {
>
> /**
> * struct dpu_rm_hw_iter - iterator for use with dpu_rm
> - * @hw: dpu_hw object requested, or NULL on failure
> * @blk: dpu_rm internal block representation. Clients ignore. Used as
> iterator.
> * @type: Hardware Block Type client wishes to search for.
> */
> struct dpu_rm_hw_iter {
> - struct dpu_hw_blk *hw;
> struct dpu_rm_hw_blk *blk;
> enum dpu_hw_blk_type type;
> };
> @@ -74,7 +72,6 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm, struct
> dpu_rm_hw_iter *i)
> return false;
> }
>
> - i->hw = NULL;
> blk_list = >hw_blks[i->type];
>
> if (i->blk && (>blk->list == blk_list)) {
> @@ -84,12 +81,9 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm,
> struct dpu_rm_hw_iter *i)
>
> i->blk = list_prepare_entry(i->blk, blk_list, list);
>
> - list_for_each_entry_continue(i->blk, blk_list, list) {
> - if (!i->blk->in_use) {
> - i->hw = i->blk->hw;
> + list_for_each_entry_continue(i->blk, blk_list, list)
> + if (!i->blk->in_use)
> return true;
> - }
> - }
>
> return false;
> }
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 14/25] drm/msm/dpu: remove enc_id tagging for hw blocks
On Mon, Oct 08, 2018 at 09:27:31PM -0700, Jeykumar Sankaran wrote:
> RM was using encoder id's to tag HW block's to reserve
> and retrieve later for display pipeline. Now
> that all the reserved HW blocks for a display are
> maintained in its crtc state, no retrieval is needed.
> This patch cleans up RM of encoder id tagging.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 90
> +--
> drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 28 --
> 2 files changed, 36 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 303f1b3..a8461b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -21,9 +21,6 @@
> #include "dpu_encoder.h"
> #include "dpu_trace.h"
>
> -#define RESERVED_BY_OTHER(h, r) \
> - ((h)->enc_id && (h)->enc_id != r)
> -
> /**
> * struct dpu_rm_requirements - Reservation requirements parameter bundle
> * @topology: selected topology for the display
> @@ -38,12 +35,13 @@ struct dpu_rm_requirements {
> /**
> * struct dpu_rm_hw_blk - hardware block tracking list member
> * @list:List head for list of all hardware blocks tracking items
> - * @enc_id: Encoder id to which this blk is binded
> + * @in_use: True, if the hw block is assigned to a display pipeline.
> + * False, otherwise
> * @hw: Pointer to the hardware register access object for this
> block
> */
> struct dpu_rm_hw_blk {
> struct list_head list;
> - uint32_t enc_id;
> + bool in_use;
How do the reservations work for TEST_ONLY commits? At a quick glance it looks
like they might be marked in_use?
Sean
> struct dpu_hw_blk *hw;
> };
>
> @@ -51,23 +49,19 @@ struct dpu_rm_hw_blk {
> * struct dpu_rm_hw_iter - iterator for use with dpu_rm
> * @hw: dpu_hw object requested, or NULL on failure
> * @blk: dpu_rm internal block representation. Clients ignore. Used as
> iterator.
> - * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for Any
> Encoder
> * @type: Hardware Block Type client wishes to search for.
> */
> struct dpu_rm_hw_iter {
> struct dpu_hw_blk *hw;
> struct dpu_rm_hw_blk *blk;
> - uint32_t enc_id;
> enum dpu_hw_blk_type type;
> };
>
> static void _dpu_rm_init_hw_iter(
> struct dpu_rm_hw_iter *iter,
> - uint32_t enc_id,
> enum dpu_hw_blk_type type)
> {
> memset(iter, 0, sizeof(*iter));
> - iter->enc_id = enc_id;
> iter->type = type;
> }
>
> @@ -91,16 +85,12 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm,
> struct dpu_rm_hw_iter *i)
> i->blk = list_prepare_entry(i->blk, blk_list, list);
>
> list_for_each_entry_continue(i->blk, blk_list, list) {
> - if (i->enc_id == i->blk->enc_id) {
> + if (!i->blk->in_use) {
> i->hw = i->blk->hw;
> - DPU_DEBUG("found type %d id %d for enc %d\n",
> - i->type, i->blk->hw->id, i->enc_id);
> return true;
> }
> }
>
> - DPU_DEBUG("no match, type %d for enc %d\n", i->type, i->enc_id);
> -
> return false;
> }
>
> @@ -196,7 +186,6 @@ static int _dpu_rm_hw_blk_create(
> }
>
> blk->hw = hw;
> - blk->enc_id = 0;
> list_add_tail(>list, >hw_blks[type]);
>
> return 0;
> @@ -301,7 +290,6 @@ static bool _dpu_rm_needs_split_display(const struct
> msm_display_topology *top)
> * proposed use case requirements, incl. hardwired dependent blocks like
> * pingpong
> * @rm: dpu resource manager handle
> - * @enc_id: encoder id requesting for allocation
> * @reqs: proposed use case requirements
> * @lm: proposed layer mixer, function checks if lm, and all other hardwired
> * blocks connected to the lm (pp) is available and appropriate
> @@ -313,7 +301,6 @@ static bool _dpu_rm_needs_split_display(const struct
> msm_display_topology *top)
> */
> static bool _dpu_rm_check_lm_and_get_connected_blks(
> struct dpu_rm *rm,
> - uint32_t enc_id,
> struct dpu_rm_requirements *reqs,
> struct dpu_rm_hw_blk *lm,
> struct dpu_rm_hw_blk **pp,
> @@ -339,13 +326,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
> }
> }
>
> - /* Already reserved? */
> - if (RESERVED_BY_OTHER(lm, enc_id)) {
> - DPU_DEBUG("lm %d already reserved\n", lm_cfg->id);
> - return false;
> - }
> -
> - _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
> + _dpu_rm_init_hw_iter(, DPU_HW_BLK_PINGPONG);
> while (_dpu_rm_get_hw_locked(rm, )) {
> if (iter.blk->hw->id == lm_cfg->pingpong) {
> *pp = iter.blk;
> @@ -358,16 +339,10 @@ static bool
Re: [Freedreno] [PATCH 12/25] drm/msm/dpu: remove mode_set_complete
On Mon, Oct 08, 2018 at 09:27:29PM -0700, Jeykumar Sankaran wrote:
> This flag was introduced as a fix to notify modeset complete
> when hw reservations were happening in both atomic_check
> and atomic_commit paths. Now that we are reserving only in
> atomic_check, we can get rid of this flag.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 +++
> 1 file changed, 3 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dd482ca..468b8fd0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -167,7 +167,6 @@ enum dpu_enc_rc_states {
> * clks and resources after IDLE_TIMEOUT time.
> * @vsync_event_work:worker to handle vsync event for
> autorefresh
> * @topology: topology of the display
> - * @mode_set_complete: flag to indicate modeset completion
> * @idle_timeout:idle timeout duration in milliseconds
> */
> struct dpu_encoder_virt {
> @@ -204,7 +203,6 @@ struct dpu_encoder_virt {
> struct kthread_delayed_work delayed_off_work;
> struct kthread_work vsync_event_work;
> struct msm_display_topology topology;
> - bool mode_set_complete;
>
> u32 idle_timeout;
> };
> @@ -636,18 +634,9 @@ static int dpu_encoder_virt_atomic_check(
>
> topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>
> - if (!ret) {
> - /*
> - * Avoid reserving resources when mode set is pending. Topology
> - * info may not be available to complete reservation.
> - */
> - if (drm_atomic_crtc_needs_modeset(crtc_state)
> - && dpu_enc->mode_set_complete) {
> - ret = dpu_rm_reserve(_kms->rm, drm_enc, crtc_state,
> - topology, false);
> - dpu_enc->mode_set_complete = false;
> - }
> - }
> + if (!ret && drm_atomic_crtc_needs_modeset(crtc_state))
> + ret = dpu_rm_reserve(_kms->rm, drm_enc, crtc_state,
> + topology, false);
>
> if (!ret)
> drm_mode_set_crtcinfo(adj_mode, 0);
> @@ -1060,8 +1049,6 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> phys->ops.mode_set(phys, mode, adj_mode);
> }
> }
> -
> - dpu_enc->mode_set_complete = true;
> }
>
> static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 11/25] drm/msm/dpu: remove reserve in encoder mode_set
On Mon, Oct 08, 2018 at 09:27:28PM -0700, Jeykumar Sankaran wrote:
> Now that we have crtc state tracking the reserved
> HW resources, we have access to them after atomic swap.
> So avoid reserving the resources in mode_set.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 17 ++---
> 1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index a8fd14e..dd482ca 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -636,7 +636,6 @@ static int dpu_encoder_virt_atomic_check(
>
> topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
>
> - /* Reserve dynamic resources now. Indicating AtomicTest phase */
> if (!ret) {
> /*
>* Avoid reserving resources when mode set is pending. Topology
> @@ -645,7 +644,7 @@ static int dpu_encoder_virt_atomic_check(
> if (drm_atomic_crtc_needs_modeset(crtc_state)
> && dpu_enc->mode_set_complete) {
> ret = dpu_rm_reserve(_kms->rm, drm_enc, crtc_state,
> - topology, true);
> + topology, false);
> dpu_enc->mode_set_complete = false;
> }
> }
> @@ -1002,8 +1001,7 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> struct list_head *connector_list;
> struct drm_connector *conn = NULL, *conn_iter;
> struct dpu_crtc_state *dpu_cstate;
> - struct msm_display_topology topology;
> - int i = 0, ret;
> + int i = 0;
>
> if (!drm_enc) {
> DPU_ERROR("invalid encoder\n");
> @@ -1031,17 +1029,6 @@ static void dpu_encoder_virt_mode_set(struct
> drm_encoder *drm_enc,
> return;
> }
>
> - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode);
> -
> - /* Reserve dynamic resources now. Indicating non-AtomicTest phase */
> - ret = dpu_rm_reserve(_kms->rm, drm_enc, drm_enc->crtc->state,
> - topology, false);
> - if (ret) {
> - DPU_ERROR_ENC(dpu_enc,
> - "failed to reserve hw resources, %d\n", ret);
> - return;
> - }
> -
> dpu_cstate = to_dpu_crtc_state(drm_enc->crtc->state);
>
> for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 10/25] drm/msm/dpu: maintain hw_mdp in kms
On Mon, Oct 08, 2018 at 09:27:27PM -0700, Jeykumar Sankaran wrote:
> hw_mdp block is common for displays. No need
> to reserve per display.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 7 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 20
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 10 --
> 3 files changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 8309850..fdc89a8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -689,6 +689,10 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
> devm_iounmap(_kms->pdev->dev, dpu_kms->vbif[VBIF_RT]);
> dpu_kms->vbif[VBIF_RT] = NULL;
>
> + if (dpu_kms->hw_mdp)
> + dpu_hw_mdp_destroy(dpu_kms->hw_mdp);
> + dpu_kms->hw_mdp = NULL;
> +
> if (dpu_kms->mmio)
> devm_iounmap(_kms->pdev->dev, dpu_kms->mmio);
> dpu_kms->mmio = NULL;
> @@ -1083,7 +1087,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
>
> dpu_kms->rm_init = true;
>
> - dpu_kms->hw_mdp = dpu_rm_get_mdp(_kms->rm);
> + dpu_kms->hw_mdp = dpu_hw_mdptop_init(MDP_TOP, dpu_kms->mmio,
> + dpu_kms->catalog);
> if (IS_ERR_OR_NULL(dpu_kms->hw_mdp)) {
> rc = PTR_ERR(dpu_kms->hw_mdp);
> if (!dpu_kms->hw_mdp)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 24fc1c7..561120d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -63,11 +63,6 @@ struct dpu_rm_hw_iter {
> enum dpu_hw_blk_type type;
> };
>
> -struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm)
> -{
> - return rm->hw_mdp;
> -}
> -
> static void _dpu_rm_init_hw_iter(
> struct dpu_rm_hw_iter *iter,
> uint32_t enc_id,
> @@ -151,9 +146,6 @@ int dpu_rm_destroy(struct dpu_rm *rm)
> }
> }
>
> - dpu_hw_mdp_destroy(rm->hw_mdp);
> - rm->hw_mdp = NULL;
> -
> mutex_destroy(>rm_lock);
>
> return 0;
> @@ -168,11 +160,8 @@ static int _dpu_rm_hw_blk_create(
> void *hw_catalog_info)
> {
> struct dpu_rm_hw_blk *blk;
> - struct dpu_hw_mdp *hw_mdp;
> void *hw;
>
> - hw_mdp = rm->hw_mdp;
> -
> switch (type) {
> case DPU_HW_BLK_LM:
> hw = dpu_hw_lm_init(id, mmio, cat);
> @@ -236,15 +225,6 @@ int dpu_rm_init(struct dpu_rm *rm,
> for (type = 0; type < DPU_HW_BLK_MAX; type++)
> INIT_LIST_HEAD(>hw_blks[type]);
>
> - /* Some of the sub-blocks require an mdptop to be created */
> - rm->hw_mdp = dpu_hw_mdptop_init(MDP_TOP, mmio, cat);
> - if (IS_ERR_OR_NULL(rm->hw_mdp)) {
> - rc = PTR_ERR(rm->hw_mdp);
> - rm->hw_mdp = NULL;
> - DPU_ERROR("failed: mdp hw not available\n");
> - goto fail;
> - }
> -
> /* Interrogate HW catalog and create tracking items for hw blocks */
> for (i = 0; i < cat->mixer_count; i++) {
> struct dpu_lm_cfg *lm = >mixer[i];
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index c7e3b2b..7ac1553 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -24,13 +24,11 @@
> * struct dpu_rm - DPU dynamic hardware resource manager
> * @hw_blks: array of lists of hardware resources present in the system, one
> * list per type of hardware block
> - * @hw_mdp: hardware object for mdp_top
> * @lm_max_width: cached layer mixer maximum width
> * @rm_lock: resource manager mutex
> */
> struct dpu_rm {
> struct list_head hw_blks[DPU_HW_BLK_MAX];
> - struct dpu_hw_mdp *hw_mdp;
> uint32_t lm_max_width;
> struct mutex rm_lock;
> };
> @@ -82,12 +80,4 @@ int dpu_rm_reserve(struct dpu_rm *rm,
> * @Return: 0 on Success otherwise -ERROR
> */
> void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state *crtc_state);
> -
> -/**
> - * dpu_rm_get_mdp - Retrieve HW block for MDP TOP.
> - * This is never reserved, and is usable by any display.
> - * @rm: DPU Resource Manager handle
> - * @Return: Pointer to hw block or NULL
> - */
> -struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm);
> #endif /* __DPU_RM_H__ */
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 09/25] drm/msm/dpu: make RM iterator static
On Mon, Oct 08, 2018 at 09:27:26PM -0700, Jeykumar Sankaran wrote:
> HW blocks reserved for a display are stored in crtc state.
> No one outside RM is interested in using these API's for
> HW block list iterations.
>
> Signed-off-by: Jeykumar Sankaran
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 37 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 46
> --
> 2 files changed, 20 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 619b596..24fc1c7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -49,12 +49,26 @@ struct dpu_rm_hw_blk {
> struct dpu_hw_blk *hw;
> };
>
> +/**
> + * struct dpu_rm_hw_iter - iterator for use with dpu_rm
> + * @hw: dpu_hw object requested, or NULL on failure
> + * @blk: dpu_rm internal block representation. Clients ignore. Used as
> iterator.
> + * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for Any
> Encoder
> + * @type: Hardware Block Type client wishes to search for.
> + */
> +struct dpu_rm_hw_iter {
> + void *hw;
> + struct dpu_rm_hw_blk *blk;
> + uint32_t enc_id;
> + enum dpu_hw_blk_type type;
> +};
> +
> struct dpu_hw_mdp *dpu_rm_get_mdp(struct dpu_rm *rm)
> {
> return rm->hw_mdp;
> }
>
> -void dpu_rm_init_hw_iter(
> +static void _dpu_rm_init_hw_iter(
> struct dpu_rm_hw_iter *iter,
> uint32_t enc_id,
> enum dpu_hw_blk_type type)
> @@ -97,17 +111,6 @@ static bool _dpu_rm_get_hw_locked(struct dpu_rm *rm,
> struct dpu_rm_hw_iter *i)
> return false;
> }
>
> -bool dpu_rm_get_hw(struct dpu_rm *rm, struct dpu_rm_hw_iter *i)
> -{
> - bool ret;
> -
> - mutex_lock(>rm_lock);
> - ret = _dpu_rm_get_hw_locked(rm, i);
> - mutex_unlock(>rm_lock);
> -
> - return ret;
> -}
> -
> static void _dpu_rm_hw_destroy(enum dpu_hw_blk_type type, void *hw)
> {
> switch (type) {
> @@ -365,7 +368,7 @@ static bool _dpu_rm_check_lm_and_get_connected_blks(
> return false;
> }
>
> - dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
> + _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_PINGPONG);
> while (_dpu_rm_get_hw_locked(rm, )) {
> if (iter.blk->id == lm_cfg->pingpong) {
> *pp = iter.blk;
> @@ -404,7 +407,7 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm,
> uint32_t enc_id,
> }
>
> /* Find a primary mixer */
> - dpu_rm_init_hw_iter(_i, 0, DPU_HW_BLK_LM);
> + _dpu_rm_init_hw_iter(_i, 0, DPU_HW_BLK_LM);
> while (lm_count != reqs->topology.num_lm &&
> _dpu_rm_get_hw_locked(rm, _i)) {
> memset(, 0, sizeof(lm));
> @@ -421,7 +424,7 @@ static int _dpu_rm_reserve_lms(struct dpu_rm *rm,
> uint32_t enc_id,
> ++lm_count;
>
> /* Valid primary mixer found, find matching peers */
> - dpu_rm_init_hw_iter(_j, 0, DPU_HW_BLK_LM);
> + _dpu_rm_init_hw_iter(_j, 0, DPU_HW_BLK_LM);
>
> while (lm_count != reqs->topology.num_lm &&
> _dpu_rm_get_hw_locked(rm, _j)) {
> @@ -480,7 +483,7 @@ static int _dpu_rm_reserve_ctls(
>
> needs_split_display = _dpu_rm_needs_split_display(top);
>
> - dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_CTL);
> + _dpu_rm_init_hw_iter(, 0, DPU_HW_BLK_CTL);
> while (_dpu_rm_get_hw_locked(rm, )) {
> const struct dpu_hw_ctl *ctl = to_dpu_hw_ctl(iter.blk->hw);
> unsigned long features = ctl->caps->features;
> @@ -528,7 +531,7 @@ static struct dpu_rm_hw_blk *_dpu_rm_reserve_intf(
> struct dpu_rm_hw_iter iter;
>
> /* Find the block entry in the rm, and note the reservation */
> - dpu_rm_init_hw_iter(, 0, type);
> + _dpu_rm_init_hw_iter(, 0, type);
> while (_dpu_rm_get_hw_locked(rm, )) {
> if (iter.blk->id != id)
> continue;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> index e48e8f2..c7e3b2b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> @@ -36,26 +36,6 @@ struct dpu_rm {
> };
>
> /**
> - * struct dpu_rm_hw_blk - resource manager internal structure
> - * forward declaration for single iterator definition without void pointer
> - */
> -struct dpu_rm_hw_blk;
> -
> -/**
> - * struct dpu_rm_hw_iter - iterator for use with dpu_rm
> - * @hw: dpu_hw object requested, or NULL on failure
> - * @blk: dpu_rm internal block representation. Clients ignore. Used as
> iterator.
> - * @enc_id: DRM ID of Encoder client wishes to search for, or 0 for Any
> Encoder
> - * @type: Hardware Block Type client wishes to search for.
> - */
> -struct dpu_rm_hw_iter {
> - void *hw;
> - struct dpu_rm_hw_blk *blk;
> -
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On 10-10-18, 08:48, Jordan Crouse wrote: > qcom,level comes straight from: > > https://lore.kernel.org/lkml/20180627045234.27403-2-rna...@codeaurora.org/ > > But in this case instead of using the CPU to program the RPMh we are passing > the value to a microprocessor (the GMU) and that will do the vote on our > behalf > (Technically we use the value to look up the vote in the cmd-db database and > pass that to the GMU) > > This is why the qcom,level was added in the first place so we could at least > share the nomenclature with the rpmhd if not the implementation. How you actually pass the vote to the underlying hardware, RPMh or GMU, is irrelevant to the whole thing. What is important is how we describe that in DT and how we represent the whole thing. We have chosen genpd + OPP to do this and same should be used by you as well. Another benefit is that the genpd core will do vote aggregation for you here. -- viresh ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 08/25] drm/msm/dpu: release reservation using crtc state
On Mon, Oct 08, 2018 at 09:27:25PM -0700, Jeykumar Sankaran wrote:
> Use the hw block pointers stored in crtc state to
> release them back to RM resource pool. This change
> is made to uncouple RM reservation from encoder_id.
> Separate change is submitted to clean up RM of
> encoder id tagging.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 69
> +++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 6 +--
> 3 files changed, 60 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 17dbbc3..a8fd14e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1223,7 +1223,7 @@ static void dpu_encoder_virt_disable(struct drm_encoder
> *drm_enc)
>
> DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
>
> - dpu_rm_release(_kms->rm, drm_enc);
> + dpu_rm_release(_kms->rm, drm_enc->crtc->state);
From drm_encoder.h:
* @crtc: Currently bound CRTC, only really meaningful for non-atomic
* drivers. Atomic drivers should instead check
* _connector_state.crtc.
> }
>
> static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index 5703b11..619b596 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -625,27 +625,70 @@ static int _dpu_rm_populate_requirements(
> return 0;
> }
>
> -static void _dpu_rm_release_reservation(struct dpu_rm *rm, uint32_t enc_id)
> +static int _dpu_rm_release_hw(struct dpu_rm *rm, enum dpu_hw_blk_type type,
> + int id)
> {
> struct dpu_rm_hw_blk *blk;
> - enum dpu_hw_blk_type type;
>
> - for (type = 0; type < DPU_HW_BLK_MAX; type++) {
> - list_for_each_entry(blk, >hw_blks[type], list) {
> - if (blk->enc_id == enc_id) {
> - blk->enc_id = 0;
> - DPU_DEBUG("rel enc %d %d %d\n", enc_id,
> - blk->type, blk->id);
> - }
> + list_for_each_entry(blk, >hw_blks[type], list) {
> + if (blk->hw->id == id) {
> + blk->enc_id = 0;
> + return 0;
> }
> }
> +
> + DRM_DEBUG_KMS("failed to find hw id(%d) of type(%d) for releasing\n",
> + id, type);
> +
> + return -EINVAL;
> }
>
> -void dpu_rm_release(struct dpu_rm *rm, struct drm_encoder *enc)
> +static void _dpu_rm_release_reservation(struct dpu_rm *rm,
> + struct dpu_crtc_state *dpu_cstate)
> {
> + int i;
> +
> + for (i = 0; i < dpu_cstate->num_mixers; i++) {
> + struct dpu_crtc_mixer *mixer = _cstate->mixers[i];
> +
> + if (!mixer->hw_lm)
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_LM,
> + mixer->hw_lm->base.id))
> + mixer->hw_lm = NULL;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_PINGPONG,
> + mixer->hw_pp->base.id))
> + mixer->hw_pp = NULL;
> + }
> +
> + for (i = 0; i < dpu_cstate->num_ctls; i++) {
> + if (!dpu_cstate->hw_ctls[i])
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_CTL,
> + dpu_cstate->hw_ctls[i]->base.id))
> + dpu_cstate->hw_ctls[i] = NULL;
> + }
> +
> + for (i = 0; i < dpu_cstate->num_intfs; i++) {
> + if (!dpu_cstate->hw_intfs[i])
> + continue;
> +
> + if (!_dpu_rm_release_hw(rm, DPU_HW_BLK_INTF,
> + dpu_cstate->hw_intfs[i]->base.id))
> + dpu_cstate->hw_intfs[i] = NULL;
> + }
> +}
> +
> +void dpu_rm_release(struct dpu_rm *rm, struct drm_crtc_state *crtc_state)
> +{
> + struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
> +
> mutex_lock(>rm_lock);
>
> - _dpu_rm_release_reservation(rm, enc->base.id);
> + _dpu_rm_release_reservation(rm, dpu_cstate);
>
> mutex_unlock(>rm_lock);
> }
> @@ -679,12 +722,12 @@ int dpu_rm_reserve(
> ret = _dpu_rm_make_reservation(rm, enc, dpu_cstate, );
> if (ret) {
> DPU_ERROR("failed to reserve hw resources: %d\n", ret);
> - _dpu_rm_release_reservation(rm, enc->base.id);
> + _dpu_rm_release_reservation(rm, dpu_cstate);
> } else if (test_only) {
>/* test_only: test the reservation and then undo */
> DPU_DEBUG("test_only: discard test [enc:
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On Wed, Oct 10, 2018 at 08:01:49PM +0530, Viresh Kumar wrote:
> On 10-10-18, 08:29, Jordan Crouse wrote:
> > On Wed, Oct 10, 2018 at 03:16:28PM +0530, Viresh Kumar wrote:
> > > On 27-08-18, 09:11, Jordan Crouse wrote:
> > > > Add the nodes to describe the Adreno GPU and GMU devices.
> > > >
> > > > Signed-off-by: Jordan Crouse
> > > > ---
> > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 121 +++
> > > > 1 file changed, 121 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > index cdaabeb3c995..10db0ceb3699 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > > @@ -192,6 +192,59 @@
> > > > method = "smc";
> > > > };
> > > >
> > > > +gpu_opp_table: adreno-opp-table {
> > > > + compatible = "operating-points-v2-qcom-level";
> > > > +
> > > > + opp-71000 {
> > > > + opp-hz = /bits/ 64 <71000>;
> > > > + qcom,level = <416>;
> > >
> > > What is qcom,level here ? Is it different than the RPM voting thing ?
> > >
> > > If not then you need to follow what Rajendra, Niklas are doing as
> > > well. There needs to be a genpd which needs to carry this value and
> > > the gpu's table will have required-opps entry to point to it.
> >
> > I don't think it is the same (we have some special considerations here)
> > but I missed the new work from the other folks and I want to review it
> > before I conclude one way or the other. Is there a link to the latest
> > and greatest that I can use to get caught up?
>
> lkml.kernel.org/r/20180627045234.27403-1-rna...@codeaurora.org
>
> +Rajendra/Niklas, please review Jordan's work as well to see if the
> qcom,level thing is similar to what you guys are using.
qcom,level comes straight from:
https://lore.kernel.org/lkml/20180627045234.27403-2-rna...@codeaurora.org/
But in this case instead of using the CPU to program the RPMh we are passing
the value to a microprocessor (the GMU) and that will do the vote on our behalf
(Technically we use the value to look up the vote in the cmd-db database and
pass that to the GMU)
This is why the qcom,level was added in the first place so we could at least
share the nomenclature with the rpmhd if not the implementation.
Jordan
> --
> viresh
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 24/25] drm/msm/dpu: remove mutex locking for RM interfaces
On Tue, Oct 09, 2018 at 11:03:24PM -0700, Jeykumar Sankaran wrote:
> On 2018-10-09 12:57, Sean Paul wrote:
> > On Mon, Oct 08, 2018 at 09:27:41PM -0700, Jeykumar Sankaran wrote:
> > > Since HW reservations are happening through atomic_check
> > > and all the display commits are catered by a single commit thread,
> > > it is not necessary to protect the interfaces by a separate
> > > mutex.
> > >
> > > Signed-off-by: Jeykumar Sankaran
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 24
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 --
> > > 2 files changed, 26 deletions(-)
> > >
> >
> > /snip
> >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > > index 8676fa5..9acbeba 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
> > > @@ -24,11 +24,9 @@
> > > * struct dpu_rm - DPU dynamic hardware resource manager
> > > * @hw_blks: array of lists of hardware resources present in the
> > system, one
> > > * list per type of hardware block
> > > - * @rm_lock: resource manager mutex
> > > */
> > > struct dpu_rm {
> > > struct list_head hw_blks[DPU_HW_BLK_MAX];
> >
> > At this point, there's really not much point to even having the rm. It's
> > just
> > another level of indirection that IMO complicates the code. If you look
> > at the usage of hw_blks, the code is always looking at a specific type
> > of
> > hw_blk, so the array is unnecessary.
> >
> > dpu_kms could just keep a few arrays/lists of the hw types, and the crtc
> > and encoder
> > reserve functions can just go in crtc/encoder.
> >
> > Sean
> >
> RM has been reduced to its current form to manage only LM/PP, CTL and
> interfaces.
> Our eventual plan is to support all the advanced HW blocks and its features
> in
> an upstream friendly way. When RM grows to manage all its subblocks,
> iteration
> logic may get heavy since the chipset have HW chain restrictions on various
> hw blocks.
> To provide room for the growth, I suggest keeping the allocation
> helpers in a separate file. But I can see why you want to maintain the HW
> block lists
> in the KMS.
At least for the blocks that exist, using the RM is unnecessary, does that
change for the current blocks when you add more? I'm guessing their code will
remain unchanged.
If the new blocks you're adding have a lot of commonality, perhaps it makes
sense to re-introduce the RM, but IMO it doesn't make sense for lm/ctl/pp.
Sean
>
> Thanks,
> Jeykumar S.
> > > - struct mutex rm_lock;
> > > };
> > >
> > > /**
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > >
> > > ___
> > > Freedreno mailing list
> > > Freedreno@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/freedreno
>
> --
> Jeykumar S
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 22/25] drm/msm/dpu: make crtc and encoder specific HW reservation
On Tue, Oct 09, 2018 at 11:15:02PM -0700, Jeykumar Sankaran wrote:
> On 2018-10-09 13:41, Sean Paul wrote:
> > On Mon, Oct 08, 2018 at 09:27:39PM -0700, Jeykumar Sankaran wrote:
> > > Instead of letting encoder make a centralized reservation for
> > > all of its display DRM components, this path splits the
> > > responsibility between CRTC and Encoder, each requesting
> > > RM for the HW mapping of its own domain.
> > >
> > > Signed-off-by: Jeykumar Sankaran
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 31 +
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 69
> > -
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 36 +++
> > > 4 files changed, 119 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 0625f56..0536b8a 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -47,6 +47,8 @@
> > > #define LEFT_MIXER 0
> > > #define RIGHT_MIXER 1
> > >
> > > +#define MAX_VDISPLAY_SPLIT 1080
> > > +
> > > static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state
> > *cstate,
> > > struct drm_display_mode *mode)
> > > {
> > > @@ -448,6 +450,7 @@ static void _dpu_crtc_setup_lm_bounds(struct
> > drm_crtc *crtc,
> > >
> > > for (i = 0; i < cstate->num_mixers; i++) {
> > > struct drm_rect *r = >lm_bounds[i];
> > > +
> > > r->x1 = crtc_split_width * i;
> > > r->y1 = 0;
> > > r->x2 = r->x1 + crtc_split_width;
> > > @@ -885,6 +888,7 @@ static void dpu_crtc_disable(struct drm_crtc
> > > *crtc)
> > > struct drm_display_mode *mode;
> > > struct drm_encoder *encoder;
> > > struct msm_drm_private *priv;
> > > + struct dpu_kms *dpu_kms;
> > > unsigned long flags;
> > >
> > > if (!crtc || !crtc->dev || !crtc->dev->dev_private ||
> > !crtc->state) {
> > > @@ -895,6 +899,7 @@ static void dpu_crtc_disable(struct drm_crtc
> > > *crtc)
> > > cstate = to_dpu_crtc_state(crtc->state);
> > > mode = >base.adjusted_mode;
> > > priv = crtc->dev->dev_private;
> > > + dpu_kms = to_dpu_kms(priv->kms);
> > >
> > > DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
> > >
> > > @@ -953,6 +958,8 @@ static void dpu_crtc_disable(struct drm_crtc
> > > *crtc)
> > > crtc->state->event = NULL;
> > > spin_unlock_irqrestore(>dev->event_lock, flags);
> > > }
> > > +
> > > + dpu_rm_crtc_release(_kms->rm, crtc->state);
> > > }
> > >
> > > static void dpu_crtc_enable(struct drm_crtc *crtc,
> > > @@ -1004,6 +1011,21 @@ struct plane_state {
> > > u32 pipe_id;
> > > };
> > >
> > > +static void _dpu_crtc_get_topology(
> > > + struct drm_crtc_state *crtc_state,
> > > + struct drm_display_mode *mode)
> > > +{
> > > + struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
> > > +
> > > + dpu_cstate->num_mixers = (mode->vdisplay > MAX_VDISPLAY_SPLIT) ? 2
> > : 1;
> > > +
> > > + /**
> > > + * encoder->atomic_check is invoked before crtc->atomic_check.
> > > + * so dpu_cstate->num_intfs should have a non-zero value.
> > > + */
> > > + dpu_cstate->num_ctls = dpu_cstate->num_intfs;
> >
> > Why do we need num_ctls? Can't we just use dpu_cstate->num_intfs
> > directly?
> > Also,
> > you don't really need these in their own function, especially if
> > num_ctls
> > goes
> > away.
> >
> Yes. I can live with just that. But since dpu_cstate maintains HW arrays
> for each type, I thought it would be more readable if I could use
> separate variables to track their counts instead of iterating over
> ctl arrays over dpu_cstate->num_intfs and leaving comments that both
> will be same for this version of hardware.
You could change the name to make it more generic. AFAICT,
num_h_tiles == num_phys_encs == num_intfs == num_ctls
So storing it as num_h_tiles might make more sense.
>
> Also, the counts need not be the same for all the Snapdragon variants.
This is probably a good thing. It doesn't seem like the current driver would
work if these values were different, so making it explicit is a good signal that
more invasive changes are needed.
Sean
>
> Thanks,
> Jeykumar S.
> > > +}
> > > +
> > > static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
> > > struct drm_crtc_state *state)
> > > {
> > > @@ -1014,6 +1036,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc
> > *crtc,
> > > const struct drm_plane_state *pstate;
> > > struct drm_plane *plane;
> > > struct drm_display_mode *mode;
> > > + struct msm_drm_private *priv;
> > > + struct dpu_kms *dpu_kms;
> > >
> > > int cnt = 0, rc = 0, mixer_width, i, z_pos;
> > >
> > > @@ -1039,6 +1063,9 @@ static int dpu_crtc_atomic_check(struct drm_crtc
> > *crtc,
> > > goto end;
> > > }
> > >
> > > + priv =
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On 10-10-18, 08:29, Jordan Crouse wrote:
> On Wed, Oct 10, 2018 at 03:16:28PM +0530, Viresh Kumar wrote:
> > On 27-08-18, 09:11, Jordan Crouse wrote:
> > > Add the nodes to describe the Adreno GPU and GMU devices.
> > >
> > > Signed-off-by: Jordan Crouse
> > > ---
> > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 121 +++
> > > 1 file changed, 121 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index cdaabeb3c995..10db0ceb3699 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -192,6 +192,59 @@
> > > method = "smc";
> > > };
> > >
> > > +gpu_opp_table: adreno-opp-table {
> > > + compatible = "operating-points-v2-qcom-level";
> > > +
> > > + opp-71000 {
> > > + opp-hz = /bits/ 64 <71000>;
> > > + qcom,level = <416>;
> >
> > What is qcom,level here ? Is it different than the RPM voting thing ?
> >
> > If not then you need to follow what Rajendra, Niklas are doing as
> > well. There needs to be a genpd which needs to carry this value and
> > the gpu's table will have required-opps entry to point to it.
>
> I don't think it is the same (we have some special considerations here)
> but I missed the new work from the other folks and I want to review it
> before I conclude one way or the other. Is there a link to the latest
> and greatest that I can use to get caught up?
lkml.kernel.org/r/20180627045234.27403-1-rna...@codeaurora.org
+Rajendra/Niklas, please review Jordan's work as well to see if the
qcom,level thing is similar to what you guys are using.
--
viresh
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 01/25] drm/msm/dpu: fix hw ctl retrieval for mixer muxing
On Tue, Oct 09, 2018 at 10:46:41PM -0700, Jeykumar Sankaran wrote:
> On 2018-10-09 11:07, Sean Paul wrote:
> > On Mon, Oct 08, 2018 at 09:27:18PM -0700, Jeykumar Sankaran wrote:
> > > Layer mixer/pingpong block counts and hw ctl block counts
> > > will not be same for all the topologies (e.g. layer
> > > mixer muxing to single interface)
> > >
> > > Use the encoder's split_role info to retrieve the
> > > respective control path for programming.
> > >
> > > Signed-off-by: Jeykumar Sankaran
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++---
> > > 1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index 96cdf06..d12f896 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -1060,6 +1060,7 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > >
> > > for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > > struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
> > > + int ctl_index;
> > >
> > > if (phys) {
> > > if (!dpu_enc->hw_pp[i]) {
> > > @@ -1068,14 +1069,16 @@ static void dpu_encoder_virt_mode_set(struct
> > drm_encoder *drm_enc,
> > > return;
> > > }
> > >
> > > - if (!hw_ctl[i]) {
> > > + ctl_index = phys->split_role == ENC_ROLE_SLAVE ? 1
> > : 0;
> > > +
> >
> > What if MAX_CHANNELS_PER_ENC isn't 2? Similarly, what if num_phys_encs >
> > MAX_CHANNELS_PER_ENC? It seems like there should be a more formal
> > relationship
> > between all of these verious values (num_of_h_tiles assumed to be <= 2
> > as
> > well).
> > If one of them changes beyond the assumed bound, the rest of the driver
> > falls
> > over pretty hard.
> >
> MAX_CHANNELS_PER_ENC is set to 2 to represent HW limitation on the chipset
> as
> we cannot gang up more than 2 LM chain to an interface. Supporting more than
> 2
> might demand much larger changes than validating for boundaries.
>
> num_phys_enc is the max no of phys encoders we create as we are looping
> through
> num_of_h_tiles which cannot be more than priv->dsi array size.
>
> So its very unlikely we would expect these loops to go out of bound!
For now, sure. However a new revision of hardware will be a pain to add support
for if we add more assumptions, and secondly it makes it _really_ hard to
understand the code if you don't have Qualcomm employee-level access to the
hardware design :).
So this is why I'm advocating for the reduction of the number of "num_of_"
values we assume are all in the same range. It's a lot easier to understand the
hardware when you can see that a phys encoder is needed per h tile, and that a
ctl/pp is needed per phys encoder.
Anyways, just my $0.02.
Sean
>
> Thanks,
> Jeykumar S.
> >
> > > + if (!hw_ctl[ctl_index]) {
> > > DPU_ERROR_ENC(dpu_enc, "no ctl block
> > assigned"
> > > - "at idx: %d\n", i);
> > > + "at idx: %d\n", ctl_index);
> > > return;
> >
> > When you return on error here, should you give back the resources that
> > you've
> > already provisioned?
> >
> > > }
> > >
> > > phys->hw_pp = dpu_enc->hw_pp[i];
> > > - phys->hw_ctl = hw_ctl[i];
> > > + phys->hw_ctl = hw_ctl[ctl_index];
> > >
> > > phys->connector = conn->state->connector;
> > > if (phys->ops.mode_set)
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > > a Linux Foundation Collaborative Project
> > >
> > > ___
> > > Freedreno mailing list
> > > Freedreno@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/freedreno
>
> --
> Jeykumar S
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 6/9] PM / OPP: dt-bindings: Add opp-interconnect-bw
On 10-10-18, 08:27, Jordan Crouse wrote: > I'm not convinced that required-opps would work. I'm worried that the > format is too vague if we need to use it for multiple paths and possibly > other uses too, consider this: > > required-opp = <_path0_opp3, _path1_opp5, _rpmh_opp1>; > > This has ordering problems and IMO pollutes the DT namespace for no > great technical advantage. I appreciate the hesitation for opening up > the flood gates for new OPP bindings but we are entering a new era > of hyper power aware devices and some concessions will need to be made. Sure. -- viresh ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On Wed, Oct 10, 2018 at 03:16:28PM +0530, Viresh Kumar wrote:
> On 27-08-18, 09:11, Jordan Crouse wrote:
> > Add the nodes to describe the Adreno GPU and GMU devices.
> >
> > Signed-off-by: Jordan Crouse
> > ---
> > arch/arm64/boot/dts/qcom/sdm845.dtsi | 121 +++
> > 1 file changed, 121 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > index cdaabeb3c995..10db0ceb3699 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > @@ -192,6 +192,59 @@
> > method = "smc";
> > };
> >
> > +gpu_opp_table: adreno-opp-table {
> > + compatible = "operating-points-v2-qcom-level";
> > +
> > + opp-71000 {
> > + opp-hz = /bits/ 64 <71000>;
> > + qcom,level = <416>;
>
> What is qcom,level here ? Is it different than the RPM voting thing ?
>
> If not then you need to follow what Rajendra, Niklas are doing as
> well. There needs to be a genpd which needs to carry this value and
> the gpu's table will have required-opps entry to point to it.
I don't think it is the same (we have some special considerations here)
but I missed the new work from the other folks and I want to review it
before I conclude one way or the other. Is there a link to the latest
and greatest that I can use to get caught up?
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 6/9] PM / OPP: dt-bindings: Add opp-interconnect-bw
On Wed, Oct 10, 2018 at 03:29:51PM +0530, Viresh Kumar wrote:
> On 27-09-18, 11:23, Georgi Djakov wrote:
> > On 08/27/2018 06:11 PM, Jordan Crouse wrote:
> > > Add the "opp-interconnect-bw" property to specify the
> > > average and peak bandwidth for an interconnect path for
> > > a specific operating power point. A separate bandwidth
> > > pair can be specified for each of the interconnects
> > > defined for the device by appending the interconnect
> > > name to the property.
> > >
> > > Signed-off-by: Jordan Crouse
> > > ---
> > > Documentation/devicetree/bindings/opp/opp.txt | 36 +++
> > > 1 file changed, 36 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/opp/opp.txt
> > > b/Documentation/devicetree/bindings/opp/opp.txt
> > > index c396c4c0af92..d714c084f36d 100644
> > > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > > @@ -170,6 +170,11 @@ Optional properties:
> > >functioning of the current device at the current OPP (where this
> > > property is
> > >present).
> > >
> > > +- opp-interconnect-bw-: This is an array of pairs specifying the
> > > average
> > > + and peak bandwidth in bytes per second for the interconnect path known
> > > by
> > > + 'name'. This should match the name(s) specified by interconnect-names
> > > in the
> > > + device definition.
> > > +
> > > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states
> > > together.
> > >
> > > / {
> > > @@ -543,3 +548,34 @@ Example 6: opp-microvolt-, opp-microamp-:
> > > };
> > > };
> > > };
> > > +
> > > +Example 7: opp-interconnect-bw:
> > > +(example: leaf device with frequency and BW quotas)
> > > +
> > > +/ {
> > > + soc {
> > > + gpu@500 {
> > > + ...
> > > + interconnects = < 26 512>;
> > > + interconnect-names = "port0";
> > > + ...
> > > + operating-points-v2 = <_opp_table>;
> > > + };
> > > + };
> > > +
> > > + gpu_opp_table: opp_table0 {
> > > + compatible = "operating-points-v2";
> > > +
> > > + opp-71000 {
> > > + op-hz = /bits/ 64 <71000>;
> > > + /* Set peak bandwidth @ 7216000 KB/s */
> > > + opp-interconnect-bw-port0 = /bits/ 64 <0 721600>;
> >
> > This seems a bit long. I would suggest the following instead.
> > If there is only one path:
> > /* average bandwidth = 0 KB/s, peak bandwidth = 7216000 KB/s */
> > opp-bw-KBps = <0 7216000>;
> > or
> > opp-bw-MBps = <0 7216>;
> >
> > If there are multiple paths:
> > opp-bw-KBps-port0 = <0 7216000>;
> > opp-bw-KBps-port1 = <0 100>;
> >
> > The above follows a convention similar to opp-microvolt, where at
> > runtime the platform can pick a and a matching opp-bw-KBps-
> > property. If the platform doesn't pick a specific or does
> > not match with any of the opp-bw-KBps- properties, then
> > opp-bw-KBps shall be used if present.
> > For now supporting only KBps values seems enough to cover all present
> > platforms, so we can start with this and based on the requirements of
> > future platforms we can add MBps etc.
>
> +1
>
> And yes I am fine with such bindings getting introduced to the OPP
> core.
>
> I am not sure if this would fit well, but have a look at
> "required-opps" property in OPP bindings and see if that can be used
> instead of adding new bindings here. Again, I am not sure if that
> should be done :)
I'm not convinced that required-opps would work. I'm worried that the
format is too vague if we need to use it for multiple paths and possibly
other uses too, consider this:
required-opp = <_path0_opp3, _path1_opp5, _rpmh_opp1>;
This has ordering problems and IMO pollutes the DT namespace for no
great technical advantage. I appreciate the hesitation for opening up
the flood gates for new OPP bindings but we are entering a new era
of hyper power aware devices and some concessions will need to be made.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 6/9] PM / OPP: dt-bindings: Add opp-interconnect-bw
On 27-09-18, 11:23, Georgi Djakov wrote:
> On 08/27/2018 06:11 PM, Jordan Crouse wrote:
> > Add the "opp-interconnect-bw" property to specify the
> > average and peak bandwidth for an interconnect path for
> > a specific operating power point. A separate bandwidth
> > pair can be specified for each of the interconnects
> > defined for the device by appending the interconnect
> > name to the property.
> >
> > Signed-off-by: Jordan Crouse
> > ---
> > Documentation/devicetree/bindings/opp/opp.txt | 36 +++
> > 1 file changed, 36 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/opp/opp.txt
> > b/Documentation/devicetree/bindings/opp/opp.txt
> > index c396c4c0af92..d714c084f36d 100644
> > --- a/Documentation/devicetree/bindings/opp/opp.txt
> > +++ b/Documentation/devicetree/bindings/opp/opp.txt
> > @@ -170,6 +170,11 @@ Optional properties:
> >functioning of the current device at the current OPP (where this
> > property is
> >present).
> >
> > +- opp-interconnect-bw-: This is an array of pairs specifying the
> > average
> > + and peak bandwidth in bytes per second for the interconnect path known by
> > + 'name'. This should match the name(s) specified by interconnect-names
> > in the
> > + device definition.
> > +
> > Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states
> > together.
> >
> > / {
> > @@ -543,3 +548,34 @@ Example 6: opp-microvolt-, opp-microamp-:
> > };
> > };
> > };
> > +
> > +Example 7: opp-interconnect-bw:
> > +(example: leaf device with frequency and BW quotas)
> > +
> > +/ {
> > + soc {
> > + gpu@500 {
> > + ...
> > + interconnects = < 26 512>;
> > + interconnect-names = "port0";
> > + ...
> > + operating-points-v2 = <_opp_table>;
> > + };
> > + };
> > +
> > + gpu_opp_table: opp_table0 {
> > + compatible = "operating-points-v2";
> > +
> > + opp-71000 {
> > + op-hz = /bits/ 64 <71000>;
> > + /* Set peak bandwidth @ 7216000 KB/s */
> > + opp-interconnect-bw-port0 = /bits/ 64 <0 721600>;
>
> This seems a bit long. I would suggest the following instead.
> If there is only one path:
> /* average bandwidth = 0 KB/s, peak bandwidth = 7216000 KB/s */
> opp-bw-KBps = <0 7216000>;
> or
> opp-bw-MBps = <0 7216>;
>
> If there are multiple paths:
> opp-bw-KBps-port0 = <0 7216000>;
> opp-bw-KBps-port1 = <0 100>;
>
> The above follows a convention similar to opp-microvolt, where at
> runtime the platform can pick a and a matching opp-bw-KBps-
> property. If the platform doesn't pick a specific or does
> not match with any of the opp-bw-KBps- properties, then
> opp-bw-KBps shall be used if present.
> For now supporting only KBps values seems enough to cover all present
> platforms, so we can start with this and based on the requirements of
> future platforms we can add MBps etc.
+1
And yes I am fine with such bindings getting introduced to the OPP
core.
I am not sure if this would fit well, but have a look at
"required-opps" property in OPP bindings and see if that can be used
instead of adding new bindings here. Again, I am not sure if that
should be done :)
--
viresh
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 5/9] arm64: dts: sdm845: Add gpu and gmu device nodes
On 27-08-18, 09:11, Jordan Crouse wrote:
> Add the nodes to describe the Adreno GPU and GMU devices.
>
> Signed-off-by: Jordan Crouse
> ---
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 121 +++
> 1 file changed, 121 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index cdaabeb3c995..10db0ceb3699 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -192,6 +192,59 @@
> method = "smc";
> };
>
> +gpu_opp_table: adreno-opp-table {
> + compatible = "operating-points-v2-qcom-level";
> +
> + opp-71000 {
> + opp-hz = /bits/ 64 <71000>;
> + qcom,level = <416>;
What is qcom,level here ? Is it different than the RPM voting thing ?
If not then you need to follow what Rajendra, Niklas are doing as
well. There needs to be a genpd which needs to carry this value and
the gpu's table will have required-opps entry to point to it.
--
viresh
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 3/3] dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845
Add interconnect properties such as interconnect provider specifier , the edge source and destination ports which are required by the interconnect API to configure interconnect path for MDSS. Changes in v2: -none Signed-off-by: Sravanthi Kollukuduru --- Documentation/devicetree/bindings/display/msm/dpu.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index ad2e8830324e..abd4d99b5030 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -28,6 +28,11 @@ Required properties: - #address-cells: number of address cells for the MDSS children. Should be 1. - #size-cells: Should be 1. - ranges: parent bus address space is the same as the child bus address space. +- interconnects : pairs of phandles and interconnect provider specifier to + denote the edge source and destination ports of the interconnect path. +- interconnect-names : list of interconnect path name strings sorted in the + same order as the interconnects property. Consumers drivers will use + interconnect-names to match interconnect paths with interconnect specifiers. Optional properties: - assigned-clocks: list of clock specifiers for clocks needing rate assignment @@ -86,6 +91,9 @@ Example: interrupt-controller; #interrupt-cells = <1>; + interconnects = < 38 512>; + interconnect-names = "port0"; + iommus = <_iommu 0>; #address-cells = <2>; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 1/3] drm/msm/dpu: clean up references of DPU custom bus scaling
Since the upstream interconnect bus framework has landed
upstream, the existing references of custom bus scaling
needs to be cleaned up.
Changes in v2:
- Fixed build error due to partial clean up
Signed-off-by: Sravanthi Kollukuduru
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c| 157 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h| 4 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 47 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 68 --
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h| 21 +--
6 files changed, 86 insertions(+), 224 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 41c5191f9056..4ee6f0dd14f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -90,7 +90,6 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
struct dpu_core_perf_params *perf)
{
struct dpu_crtc_state *dpu_cstate;
- int i;
if (!kms || !kms->catalog || !crtc || !state || !perf) {
DPU_ERROR("invalid parameters\n");
@@ -101,35 +100,24 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
memset(perf, 0, sizeof(struct dpu_core_perf_params));
if (!dpu_cstate->bw_control) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
- perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
+ perf->bw_ctl = kms->catalog->perf.max_bw_high *
1000ULL;
- perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
- }
+ perf->max_per_pipe_ib = perf->bw_ctl;
perf->core_clk_rate = kms->perf.max_core_clk_rate;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
- perf->bw_ctl[i] = 0;
- perf->max_per_pipe_ib[i] = 0;
- }
+ perf->bw_ctl = 0;
+ perf->max_per_pipe_ib = 0;
perf->core_clk_rate = 0;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
- perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
- perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
- }
+ perf->bw_ctl = kms->perf.fix_core_ab_vote;
+ perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote;
perf->core_clk_rate = kms->perf.fix_core_clk_rate;
}
DPU_DEBUG(
- "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu
llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
+ "crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu\n",
crtc->base.id, perf->core_clk_rate,
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MNOC],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
+ perf->max_per_pipe_ib, perf->bw_ctl);
}
int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -142,7 +130,6 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
struct dpu_crtc_state *dpu_cstate;
struct drm_crtc *tmp_crtc;
struct dpu_kms *kms;
- int i;
if (!crtc || !state) {
DPU_ERROR("invalid crtc\n");
@@ -164,31 +151,28 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
/* obtain new values */
_dpu_core_perf_calc_crtc(kms, crtc, state, _cstate->new_perf);
- for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
- i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
- bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
- curr_client_type = dpu_crtc_get_client_type(crtc);
+ bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl;
+ curr_client_type = dpu_crtc_get_client_type(crtc);
- drm_for_each_crtc(tmp_crtc, crtc->dev) {
- if (_dpu_core_perf_crtc_is_power_on(tmp_crtc) &&
- (dpu_crtc_get_client_type(tmp_crtc) ==
- curr_client_type) &&
- (tmp_crtc != crtc)) {
- struct dpu_crtc_state *tmp_cstate =
- to_dpu_crtc_state(tmp_crtc->state);
-
- DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n",
-
[Freedreno] [PATCH v2 0/3] Use interconnect API in MDSS on SDM845
The interconnect API provides an interface for consumer drivers to express their bandwidth needs in the SoC. This data is aggregated and the on-chip interconnect hardware is configured to the appropriate power/performance profile. MDSS is one of the interconnect consumers which uses the interconnect APIs to get the path between endpoints and set its bandwidth requirements for the given interconnected path. Subsequently, there is a clean up patch to remove all the references of the DPU custom bus scaling. There is corresponding DT patch with the source and destination ports defined for display driver which will be sent separately. Changes in v2: - Remove error log and unnecessary check (Jordan Crouse) - Fixed build error due to partial clean up Sravanthi Kollukuduru (3): drm/msm/dpu: clean up references of DPU custom bus scaling drm/msm/dpu: Integrate interconnect API in MDSS dt-bindings: msm/disp: Introduce interconnect bindings for MDSS on SDM845 .../devicetree/bindings/display/msm/dpu.txt| 8 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 157 + drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 13 +- drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 47 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 68 - drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 21 +-- 8 files changed, 140 insertions(+), 228 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 2/3] drm/msm/dpu: Integrate interconnect API in MDSS
The interconnect framework is designed to provide a
standard kernel interface to control the settings of
the interconnects on a SoC.
The interconnect API uses a consumer/provider-based model,
where the providers are the interconnect buses and the
consumers could be various drivers.
MDSS is one of the interconnect consumers which uses the
interconnect APIs to get the path between endpoints and
set its bandwidth/latency/QoS requirements for the given
interconnected path.
Changes in v2:
- Remove error log and unnecessary check (Jordan Crouse)
Signed-off-by: Sravanthi Kollukuduru
---
drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 50 +---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
index 2235ef8129f4..27c2594e5133 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c
@@ -4,6 +4,7 @@
*/
#include "dpu_kms.h"
+#include
#define to_dpu_mdss(x) container_of(x, struct dpu_mdss, base)
@@ -16,8 +17,33 @@ struct dpu_mdss {
u32 hwversion;
struct dss_module_power mp;
struct dpu_irq_controller irq_controller;
+ struct icc_path *path[2];
+ u32 num_paths;
};
+static int dpu_mdss_parse_data_bus_icc_path(
+ struct drm_device *dev, struct dpu_mdss *dpu_mdss)
+{
+ struct icc_path *path0 = of_icc_get(dev->dev, "port0");
+ struct icc_path *path1 = of_icc_get(dev->dev, "port1");
+ int total_num_paths = 0;
+
+ if (IS_ERR(path0))
+ return PTR_ERR(path0);
+
+ dpu_mdss->path[0] = path0;
+ total_num_paths = 1;
+
+ if (!IS_ERR(path1)) {
+ dpu_mdss->path[1] = path1;
+ total_num_paths++;
+ }
+
+ dpu_mdss->num_paths = total_num_paths;
+
+ return 0;
+}
+
static irqreturn_t dpu_mdss_irq(int irq, void *arg)
{
struct dpu_mdss *dpu_mdss = arg;
@@ -127,7 +153,12 @@ static int dpu_mdss_enable(struct msm_mdss *mdss)
{
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = _mdss->mp;
- int ret;
+ int ret, i;
+ u64 ab = (dpu_mdss->num_paths) ? 68/dpu_mdss->num_paths : 0;
+ u64 ib = 68;
+
+ for (i = 0; i < dpu_mdss->num_paths; i++)
+ icc_set(dpu_mdss->path[i], ab, ib);
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, true);
if (ret)
@@ -140,12 +171,15 @@ static int dpu_mdss_disable(struct msm_mdss *mdss)
{
struct dpu_mdss *dpu_mdss = to_dpu_mdss(mdss);
struct dss_module_power *mp = _mdss->mp;
- int ret;
+ int ret, i;
ret = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
if (ret)
DPU_ERROR("clock disable failed, ret:%d\n", ret);
+ for (i = 0; i < dpu_mdss->num_paths; i++)
+ icc_set(dpu_mdss->path[i], 0, 0);
+
return ret;
}
@@ -155,6 +189,7 @@ static void dpu_mdss_destroy(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct dpu_mdss *dpu_mdss = to_dpu_mdss(priv->mdss);
struct dss_module_power *mp = _mdss->mp;
+ int i;
_dpu_mdss_irq_domain_fini(dpu_mdss);
@@ -163,6 +198,9 @@ static void dpu_mdss_destroy(struct drm_device *dev)
msm_dss_put_clk(mp->clk_config, mp->num_clk);
devm_kfree(>dev, mp->clk_config);
+ for (i = 0; i < dpu_mdss->num_paths; i++)
+ icc_put(dpu_mdss->path[i]);
+
if (dpu_mdss->mmio)
devm_iounmap(>dev, dpu_mdss->mmio);
dpu_mdss->mmio = NULL;
@@ -203,6 +241,10 @@ int dpu_mdss_init(struct drm_device *dev)
}
dpu_mdss->mmio_len = resource_size(res);
+ ret = dpu_mdss_parse_data_bus_icc_path(dev, dpu_mdss);
+ if (ret)
+ return ret;
+
mp = _mdss->mp;
ret = msm_dss_parse_clock(pdev, mp);
if (ret) {
@@ -224,14 +266,14 @@ int dpu_mdss_init(struct drm_device *dev)
goto irq_error;
}
+ priv->mdss = _mdss->base;
+
pm_runtime_enable(dev->dev);
pm_runtime_get_sync(dev->dev);
dpu_mdss->hwversion = readl_relaxed(dpu_mdss->mmio);
pm_runtime_put_sync(dev->dev);
- priv->mdss = _mdss->base;
-
return ret;
irq_error:
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 07/25] drm/msm/dpu: reserve using crtc state
On 2018-10-09 14:06, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:24PM -0700, Jeykumar Sankaran wrote:
DPU maintained reservation lists to cache assigned
HW blocks for the display and a retrieval mechanism for
the individual DRM components to query their respective
HW blocks.
This patch uses the sub-classed CRTC state to store
and track HW blocks assigned for different components
of the display pipeline. It helps the driver:
- to get rid of unwanted store and retrieval RM API's
- to preserve HW resources assigned in atomic_check
through atomic swap/duplicate.
Separate patch is submitted to remove resource
reservation in atomic_commit path.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 65
+++---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 14 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 28 +++---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 20 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 58
---
5 files changed, 72 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 4960641..0625f56 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -421,69 +421,20 @@ void dpu_crtc_complete_commit(struct drm_crtc
*crtc,
trace_dpu_crtc_complete_commit(DRMID(crtc));
}
-static void _dpu_crtc_setup_mixer_for_encoder(
- struct drm_crtc *crtc,
- struct drm_encoder *enc)
+static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
{
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
- struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
- struct dpu_rm *rm = _kms->rm;
struct dpu_crtc_mixer *mixer;
- struct dpu_hw_ctl *last_valid_ctl = NULL;
- int i;
- struct dpu_rm_hw_iter lm_iter, ctl_iter;
-
- dpu_rm_init_hw_iter(_iter, enc->base.id, DPU_HW_BLK_LM);
- dpu_rm_init_hw_iter(_iter, enc->base.id, DPU_HW_BLK_CTL);
+ int i, ctl_index;
/* Set up all the mixers and ctls reserved by this encoder */
- for (i = cstate->num_mixers; i < ARRAY_SIZE(cstate->mixers); i++)
{
+ for (i = 0; i < cstate->num_mixers; i++) {
mixer = >mixers[i];
- if (!dpu_rm_get_hw(rm, _iter))
- break;
- mixer->hw_lm = (struct dpu_hw_mixer *)lm_iter.hw;
-
/* CTL may be <= LMs, if <, multiple LMs controlled by 1
CTL */
- if (!dpu_rm_get_hw(rm, _iter)) {
- DPU_DEBUG("no ctl assigned to lm %d, using
previous\n",
- mixer->hw_lm->idx - LM_0);
- mixer->lm_ctl = last_valid_ctl;
- } else {
- mixer->lm_ctl = (struct dpu_hw_ctl *)ctl_iter.hw;
- last_valid_ctl = mixer->lm_ctl;
- }
-
- /* Shouldn't happen, mixers are always >= ctls */
- if (!mixer->lm_ctl) {
- DPU_ERROR("no valid ctls found for lm %d\n",
- mixer->hw_lm->idx - LM_0);
- return;
- }
-
- cstate->num_mixers++;
- DPU_DEBUG("setup mixer %d: lm %d\n",
- i, mixer->hw_lm->idx - LM_0);
- DPU_DEBUG("setup mixer %d: ctl %d\n",
- i, mixer->lm_ctl->idx - CTL_0);
- }
-}
-
-static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
-{
- struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
- struct drm_encoder *enc;
-
- mutex_lock(_crtc->crtc_lock);
- /* Check for mixers on all encoders attached to this crtc */
- list_for_each_entry(enc, >dev->mode_config.encoder_list,
head) {
- if (enc->crtc != crtc)
- continue;
-
- _dpu_crtc_setup_mixer_for_encoder(crtc, enc);
+ ctl_index = min(i, cstate->num_ctls - 1);
This is another one of those places I mentioned where we're just
assuming
a
value is going to be in a certain range. If
num_ctls/num_intfs/num_phys_encs
(all the same value afaict) is 0, we end up in a bad place.
Even though all these variables have the same value, they are
representing the
sizes of logically seperate components.
At a minimum, there should be a WARN_ON/BUG_ON somewhere ensuring this
can
never
drop below 0.
Isn't RM guaranteeing that? I can add the WARN_ON checks on these
num_xxx when the HW blocks are allocated.
Thanks,
Jeykumar S.
+ mixer->lm_ctl = cstate->hw_ctls[ctl_index];
}
-
- mutex_unlock(_crtc->crtc_lock);
}
static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
@@ -536,10 +487,8 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
dev = crtc->dev;
Re: [Freedreno] [PATCH 22/25] drm/msm/dpu: make crtc and encoder specific HW reservation
On 2018-10-09 13:41, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:39PM -0700, Jeykumar Sankaran wrote:
Instead of letting encoder make a centralized reservation for
all of its display DRM components, this path splits the
responsibility between CRTC and Encoder, each requesting
RM for the HW mapping of its own domain.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 31 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 69
-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 36 +++
4 files changed, 119 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0625f56..0536b8a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -47,6 +47,8 @@
#define LEFT_MIXER 0
#define RIGHT_MIXER 1
+#define MAX_VDISPLAY_SPLIT 1080
+
static inline int _dpu_crtc_get_mixer_width(struct dpu_crtc_state
*cstate,
struct drm_display_mode *mode)
{
@@ -448,6 +450,7 @@ static void _dpu_crtc_setup_lm_bounds(struct
drm_crtc *crtc,
for (i = 0; i < cstate->num_mixers; i++) {
struct drm_rect *r = >lm_bounds[i];
+
r->x1 = crtc_split_width * i;
r->y1 = 0;
r->x2 = r->x1 + crtc_split_width;
@@ -885,6 +888,7 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
struct drm_display_mode *mode;
struct drm_encoder *encoder;
struct msm_drm_private *priv;
+ struct dpu_kms *dpu_kms;
unsigned long flags;
if (!crtc || !crtc->dev || !crtc->dev->dev_private ||
!crtc->state) {
@@ -895,6 +899,7 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
cstate = to_dpu_crtc_state(crtc->state);
mode = >base.adjusted_mode;
priv = crtc->dev->dev_private;
+ dpu_kms = to_dpu_kms(priv->kms);
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
@@ -953,6 +958,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
crtc->state->event = NULL;
spin_unlock_irqrestore(>dev->event_lock, flags);
}
+
+ dpu_rm_crtc_release(_kms->rm, crtc->state);
}
static void dpu_crtc_enable(struct drm_crtc *crtc,
@@ -1004,6 +1011,21 @@ struct plane_state {
u32 pipe_id;
};
+static void _dpu_crtc_get_topology(
+ struct drm_crtc_state *crtc_state,
+ struct drm_display_mode *mode)
+{
+ struct dpu_crtc_state *dpu_cstate = to_dpu_crtc_state(crtc_state);
+
+ dpu_cstate->num_mixers = (mode->vdisplay > MAX_VDISPLAY_SPLIT) ? 2
: 1;
+
+ /**
+* encoder->atomic_check is invoked before crtc->atomic_check.
+* so dpu_cstate->num_intfs should have a non-zero value.
+*/
+ dpu_cstate->num_ctls = dpu_cstate->num_intfs;
Why do we need num_ctls? Can't we just use dpu_cstate->num_intfs
directly?
Also,
you don't really need these in their own function, especially if
num_ctls
goes
away.
Yes. I can live with just that. But since dpu_cstate maintains HW arrays
for each type, I thought it would be more readable if I could use
separate variables to track their counts instead of iterating over
ctl arrays over dpu_cstate->num_intfs and leaving comments that both
will be same for this version of hardware.
Also, the counts need not be the same for all the Snapdragon variants.
Thanks,
Jeykumar S.
+}
+
static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
@@ -1014,6 +1036,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
const struct drm_plane_state *pstate;
struct drm_plane *plane;
struct drm_display_mode *mode;
+ struct msm_drm_private *priv;
+ struct dpu_kms *dpu_kms;
int cnt = 0, rc = 0, mixer_width, i, z_pos;
@@ -1039,6 +1063,9 @@ static int dpu_crtc_atomic_check(struct drm_crtc
*crtc,
goto end;
}
+ priv = crtc->dev->dev_private;
+ dpu_kms = to_dpu_kms(priv->kms);
+
mode = >adjusted_mode;
DPU_DEBUG("%s: check", dpu_crtc->name);
@@ -1229,6 +1256,10 @@ static int dpu_crtc_atomic_check(struct
drm_crtc
*crtc,
}
}
+ _dpu_crtc_get_topology(state, mode);
+ if (drm_atomic_crtc_needs_modeset(state))
+ rc = dpu_rm_crtc_reserve(_kms->rm, state);
+
end:
kfree(pstates);
return rc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5d501c8..ce66309 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -67,8 +67,6 @@
#define IDLE_SHORT_TIMEOUT 1
-#define MAX_VDISPLAY_SPLIT 1080
-
/**
* enum dpu_enc_rc_events - events for resource
Re: [Freedreno] [PATCH 24/25] drm/msm/dpu: remove mutex locking for RM interfaces
On 2018-10-09 12:57, Sean Paul wrote:
On Mon, Oct 08, 2018 at 09:27:41PM -0700, Jeykumar Sankaran wrote:
Since HW reservations are happening through atomic_check
and all the display commits are catered by a single commit thread,
it is not necessary to protect the interfaces by a separate
mutex.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 24
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 2 --
2 files changed, 26 deletions(-)
/snip
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
index 8676fa5..9acbeba 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
@@ -24,11 +24,9 @@
* struct dpu_rm - DPU dynamic hardware resource manager
* @hw_blks: array of lists of hardware resources present in the
system, one
* list per type of hardware block
- * @rm_lock: resource manager mutex
*/
struct dpu_rm {
struct list_head hw_blks[DPU_HW_BLK_MAX];
At this point, there's really not much point to even having the rm.
It's
just
another level of indirection that IMO complicates the code. If you look
at the usage of hw_blks, the code is always looking at a specific type
of
hw_blk, so the array is unnecessary.
dpu_kms could just keep a few arrays/lists of the hw types, and the
crtc
and encoder
reserve functions can just go in crtc/encoder.
Sean
RM has been reduced to its current form to manage only LM/PP, CTL and
interfaces.
Our eventual plan is to support all the advanced HW blocks and its
features in
an upstream friendly way. When RM grows to manage all its subblocks,
iteration
logic may get heavy since the chipset have HW chain restrictions on
various hw blocks.
To provide room for the growth, I suggest keeping the allocation
helpers in a separate file. But I can see why you want to maintain the
HW block lists
in the KMS.
Thanks,
Jeykumar S.
- struct mutex rm_lock;
};
/**
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
--
Jeykumar S
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
