Re: [RFC] Using DC in amdgpu for upcoming GPU

[Daniel Vetter]

On Mon, Dec 12, 2016 at 11:10:30PM -0500, Cheng, Tony wrote:
> Thanks for the write up for the guide.  We can definitely re-do atomic
> according to guideline provided as I am not satified with how our code look
> today.  To me it seems more like we need to shuffle stuff around and rename
> a few things than rewrite much of anything.
> 
> I hope to get an answer on the reply to Dave's question regarding to if
> there is anything else.  If we can keep most of the stuff under /dc as the
> "back end" helper and do most of the change under /amdgpu_dm then it isn't
> that difficult as we don't need to go deal with the fall out on other
> platforms.  Again it's not just windows.  We are fully aware that it's hard
> to find the common abstraction between all different OS so we try our best
> to have DC behave more like a helper than abstraction layer anyways.  In our
> design states and policies are domain of Display Managers (DM) and because
> of linux we also say anything DRM can do that's also domain of DM.  We don't
> put anything in DC that we don't feel comfortable if HW decide to hide it in
> FW.
> 
> 
> On 12/12/2016 9:33 PM, Harry Wentland wrote:
> > On 2016-12-11 03:28 PM, Daniel Vetter wrote:
> > > On Wed, Dec 07, 2016 at 09:02:13PM -0500, Harry Wentland wrote:
> > > > We propose to use the Display Core (DC) driver for display support on
> > > > AMD's upcoming GPU (referred to by uGPU in the rest of the doc).
> > > > In order to
> > > > avoid a flag day the plan is to only support uGPU initially and
> > > > transition
> > > > to older ASICs gradually.
> > > 
> > > Bridgeman brought it up a few times that this here was the question
> > > - it's
> > > kinda missing a question mark, hard to figure this out ;-). I'd say for
> > 
> > My bad for the missing question mark (imprecise phrasing). On the other
> > hand letting this blow over a bit helped get us on the map a bit more
> > and allows us to argue the challenges (and benefits) of open source. :)
> > 
> > > upstream it doesn't really matter, but imo having both atomic and
> > > non-atomic paths in one driver is one world of hurt and I strongly
> > > recommend against it, at least if feasible. All drivers that switched
> > > switched in one go, the only exception was i915 (it took much longer
> > > than
> > > we ever feared, causing lots of pain) and nouveau (which only converted
> > > nv50+, but pre/post-nv50 have always been two almost completely separate
> > > worlds anyway).
> > > 
> > 
> trust me we would like to upstream everything.  Just we didn't invest enough
> in DC code in the previous generation so the quality might not be there.
> 
> > You mention the two probably most complex DRM drivers didn't switch in a
> > single go...  I imagine amdgpu/DC falls into the same category.
> > 
> > I think one of the problems is making a sudden change with a fully
> > validated driver without breaking existing use cases and customers. We
> > really should've started DC development in public and probably would do
> > that if we had to start anew.
> > 
> > > > The DC component has received extensive testing within AMD for
> > > > DCE8, 10, and
> > > > 11 GPUs and is being prepared for uGPU. Support should be better than
> > > > amdgpu's current display support.
> > > > 
> > > >  * All of our QA effort is focused on DC
> > > >  * All of our CQE effort is focused on DC
> > > >  * All of our OEM preloads and custom engagements use DC
> > > >  * DC behavior mirrors what we do for other OSes
> > > > 
> > > > The new asic utilizes a completely re-designed atom interface,
> > > > so we cannot
> > > > easily leverage much of the existing atom-based code.
> > > > 
> > > > We've introduced DC to the community earlier in 2016 and
> > > > received a fair
> > > > amount of feedback. Some of what we've addressed so far are:
> > > > 
> > > >  * Self-contain ASIC specific code. We did a bunch of work to pull
> > > >common sequences into dc/dce and leave ASIC specific code in
> > > >separate folders.
> > > >  * Started to expose AUX and I2C through generic kernel/drm
> > > >functionality and are mostly using that. Some of that code is still
> > > >needlessly convoluted. This cleanup is in progress.
> > > >  * Integrated Dave and Jerome’s work on removing abstraction in bios
> > > >parser.
> > > >  * Retire adapter service and asic capability
> > > >  * Remove some abstraction in GPIO
> > > > 
> > > > Since a lot of our code is shared with pre- and post-silicon validation
> > > > suites changes need to be done gradually to prevent breakages
> > > > due to a major
> > > > flag day.  This, coupled with adding support for new asics and
> > > > lots of new
> > > > feature introductions means progress has not been as quick as we
> > > > would have
> > > > liked. We have made a lot of progress none the less.
> > > > 
> > > > The remaining concerns that were brought up during the last
> > > > review that we
> > > > are working on addressing:
> > > > 
> > > >  * 

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Dave Airlie]

(hit send too early)
> We would love to upstream DC for all supported asic!  We made enough change
> to make Sea Island work but it's really not validate to the extend we
> validate Polaris on linux and no where close to what we do for 2017 ASICs.
> With DC the display hardware programming, resource optimization, power
> management and interaction with rest of system will be fully validated
> across multiple OSs.  Therefore we have high confidence that the quality is
> going to better than what we have upstreammed today.
>
> I don't have a baseline to say if DC is in good enough quality for older
> generation compare to upstream.  For example we don't have HW generate
> bandwidth_calc for DCE 8/10 (Sea/Vocanic island family) but our code is
> structured in a way that we assume bandwidth_calc is there.  None of us feel
> like go untangle the formulas in windows driver at this point to create our
> own version of bandwidth_calc.  It sort of work with HW default values but
> some mode / config is likely to underflows.  If community is okay with
> uncertain quality, sure we would love to upstream everything to reduce our
> maintaince overhead.  You do get audio with DC on DCE8 though.

If we get any of this upstream, we should get all of the hw supported with it.

If it regresses we just need someone to debug why.

> Maybe let me share what we are doing and see if we can come up with
> something to make DC work for both upstream and our internal need.  We are
> sharing code not just on Linux and we will do our best to make our code
> upstream friendly.  Last year we focussed on having enough code to prove
> that our DAL rewrite works and get more people contributing to it.  We rush
> a bit as a result we had a few legacy component we port from Windows driver
> and we know it's bloat that needed to go.
>
> We designed DC so HW can contribute bandwidth_calc magic and psuedo code to
> program the HW blocks.  The HW blocks on the bottom of DC.JPG in models our
> HW blocks and the programming sequence are provided by HW engineers.  If a
> piece of HW need a bit toggled 7 times during power up I rather have HW
> engineer put that in their psedo code rather than me trying to find that
> sequence in some document.  Afterall they did simulate the HW with the
> toggle sequence.  I guess these are back-end code Daniel talked about.  Can
> we agree that DRM core is not interested in how things are done in that
> layer and we can upstream these as it?
>
> The next is dce_hwseq.c to program the HW blocks in correct sequence.  Some
> HW block can be programmed in any sequence, but some requires strict
> sequence to be followed.  For example Display CLK and PHY CLK need to be up
> before we enable timing generator.  I would like these sequence to remain in
> DC as it's really not DRM's business to know how to program the HW.  In a
> way you can consider hwseq as a helper to commit state to HW.
>
> Above hwseq is the dce*_resource.c.  It's job is to come up with the HW
> state required to realize given config.  For example we would use the exact
> same HW resources with same optimization setting to drive any same given
> config.  If 4 x 4k@60 is supported with resource setting A on HW diagnositc
> suite during bring up setting B on Linux then we have a problem.  It know
> which HW block work with which block and their capability and limitations.
> I hope you are not asking this stuff to move up to core because in reality
> we should probably hide this in some FW, as HW expose the register to config
> them differently that doesn't mean all combination of HW usage is validated.
> To me resource is more of a helper to put together functional pipeline and
> does not make any decision that any OS might be interested in.
>
> These yellow boxes in DC.JPG are really specific to each generation of HW
> and changes frequently.  These are things that HW has consider hiding it in
> FW before.  Can we agree on those code (under /dc/dce*) can stay?

I think most of these things are fine to be part of the solution we end up at,
but I can't say for certain they won't require interface changes. I think the
most useful code is probably the stuff in the dce subdirectories.

>
> Is this about demonstration how basic functionality work and add more
> features with series of patches to make review eaiser?  If so I don't think
> we are staff to do this kind of rewrite.  For example it make no sense to
> hooking up bandwidth_calc to calculate HW magic if we don't have mem_input
> to program the memory settings.  We need portion of hw_seq to ensure these
> blocks are programming in correct sequence.  We will need to feed
> bandwidth_calc it's required inputs, which is basically the whole system
> state tracked in validate_context today, which means we basically need big
> bulk of resource.c.  This effort might have benefit in reviewing the code,
> but we will end up with pretty much similar if not the same as what we
> already have.

This is 

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Dave Airlie]

> We would love to upstream DC for all supported asic!  We made enough change
> to make Sea Island work but it's really not validate to the extend we
> validate Polaris on linux and no where close to what we do for 2017 ASICs.
> With DC the display hardware programming, resource optimization, power
> management and interaction with rest of system will be fully validated
> across multiple OSs.  Therefore we have high confidence that the quality is
> going to better than what we have upstreammed today.
>
> I don't have a baseline to say if DC is in good enough quality for older
> generation compare to upstream.  For example we don't have HW generate
> bandwidth_calc for DCE 8/10 (Sea/Vocanic island family) but our code is
> structured in a way that we assume bandwidth_calc is there.  None of us feel
> like go untangle the formulas in windows driver at this point to create our
> own version of bandwidth_calc.  It sort of work with HW default values but
> some mode / config is likely to underflows.  If community is okay with
> uncertain quality, sure we would love to upstream everything to reduce our
> maintaince overhead.  You do get audio with DC on DCE8 though.

If we get any of this upstream, we should get all of the hw supported with it.

If it regresses we just need someone to debug why.

> Maybe let me share what we are doing and see if we can come up with
> something to make DC work for both upstream and our internal need.  We are
> sharing code not just on Linux and we will do our best to make our code
> upstream friendly.  Last year we focussed on having enough code to prove
> that our DAL rewrite works and get more people contributing to it.  We rush
> a bit as a result we had a few legacy component we port from Windows driver
> and we know it's bloat that needed to go.
>
> We designed DC so HW can contribute bandwidth_calc magic and psuedo code to
> program the HW blocks.  The HW blocks on the bottom of DC.JPG in models our
> HW blocks and the programming sequence are provided by HW engineers.  If a
> piece of HW need a bit toggled 7 times during power up I rather have HW
> engineer put that in their psedo code rather than me trying to find that
> sequence in some document.  Afterall they did simulate the HW with the
> toggle sequence.  I guess these are back-end code Daniel talked about.  Can
> we agree that DRM core is not interested in how things are done in that
> layer and we can upstream these as it?
>
> The next is dce_hwseq.c to program the HW blocks in correct sequence.  Some
> HW block can be programmed in any sequence, but some requires strict
> sequence to be followed.  For example Display CLK and PHY CLK need to be up
> before we enable timing generator.  I would like these sequence to remain in
> DC as it's really not DRM's business to know how to program the HW.  In a
> way you can consider hwseq as a helper to commit state to HW.
>
> Above hwseq is the dce*_resource.c.  It's job is to come up with the HW
> state required to realize given config.  For example we would use the exact
> same HW resources with same optimization setting to drive any same given
> config.  If 4 x 4k@60 is supported with resource setting A on HW diagnositc
> suite during bring up setting B on Linux then we have a problem.  It know
> which HW block work with which block and their capability and limitations.
> I hope you are not asking this stuff to move up to core because in reality
> we should probably hide this in some FW, as HW expose the register to config
> them differently that doesn't mean all combination of HW usage is validated.
> To me resource is more of a helper to put together functional pipeline and
> does not make any decision that any OS might be interested in.
>
> These yellow boxes in DC.JPG are really specific to each generation of HW
> and changes frequently.  These are things that HW has consider hiding it in
> FW before.  Can we agree on those code (under /dc/dce*) can stay?

I think most of these things are fine to be part of the solution we end up at,
but I can't say for certain they won't require interface changes. I think the
most useful code is probably the stuff in the dce subdirectories.

>
> Is this about demonstration how basic functionality work and add more
> features with series of patches to make review eaiser?  If so I don't think
> we are staff to do this kind of rewrite.  For example it make no sense to
> hooking up bandwidth_calc to calculate HW magic if we don't have mem_input
> to program the memory settings.  We need portion of hw_seq to ensure these
> blocks are programming in correct sequence.  We will need to feed
> bandwidth_calc it's required inputs, which is basically the whole system
> state tracked in validate_context today, which means we basically need big
> bulk of resource.c.  This effort might have benefit in reviewing the code,
> but we will end up with pretty much similar if not the same as what we
> already have.

This is something people always say, 

Re: some minor dal cleanups and example of future patches

[Dave Airlie]

On 13 December 2016 at 16:41, Dave Airlie  wrote:
> So whenever I look at DAL I find myself trying to figure out what
> all of it does, and this leads me to deleting things, so I'm just
> sending out some stuff that I cleaned today.
>
> One thing I noticed in passing is the displayport code seems
> possibly endian unsafe, it casts bytes read from hardware into
> bitfields, this is the best defined behaviour and is generally
> a pattern we try to steer away from.
>
> The porting the displayport code to use drm defines is the
> sort of example of a cleanup patch that might cause problems
> for you in code sharing, but is the exact sort of patch that
> has been applied to other drivers in the past.

I've also just noticed every dpcd access (read/write) causes
a traversal of the connector list.

Dave.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 6/8] drm/dp-helper: add missing defines needed by AMD display core.

[Dave Airlie]

From: Dave Airlie 

These are all the ones required by the AMD display core.

Signed-off-by: Dave Airlie 
---
 include/drm/drm_dp_helper.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 9d03f16..3d5910b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -400,6 +400,8 @@
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_MASK   0xc0
 # define DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT  6
 
+#define DP_ADJUST_REQUEST_POST_CURSOR2  0x20c
+
 #define DP_TEST_REQUEST0x218
 # define DP_TEST_LINK_TRAINING (1 << 0)
 # define DP_TEST_LINK_VIDEO_PATTERN(1 << 1)
@@ -415,6 +417,8 @@
 
 #define DP_TEST_PATTERN0x221
 
+#define DP_TEST_MISC1   0x232
+
 #define DP_TEST_CRC_R_CR   0x240
 #define DP_TEST_CRC_G_Y0x242
 #define DP_TEST_CRC_B_CB   0x244
@@ -423,6 +427,18 @@
 # define DP_TEST_CRC_SUPPORTED (1 << 5)
 # define DP_TEST_COUNT_MASK0xf
 
+#define DP_TEST_PHY_PATTERN 0x248
+#define DP_TEST_80BIT_CUSTOM_PATTERN_7_00x250
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_15_8   0x251
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_23_16  0x252
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_31_24  0x253
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_39_32  0x254
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_47_40  0x255
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_55_48  0x256
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_63_56  0x257
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_71_64  0x258
+#defineDP_TEST_80BIT_CUSTOM_PATTERN_79_72  0x259
+
 #define DP_TEST_RESPONSE   0x260
 # define DP_TEST_ACK   (1 << 0)
 # define DP_TEST_NAK   (1 << 1)
@@ -443,6 +459,7 @@
 #define DP_SOURCE_OUI  0x300
 #define DP_SINK_OUI0x400
 #define DP_BRANCH_OUI  0x500
+#define DP_BRANCH_REVISION_START0x509
 
 #define DP_SET_POWER0x600
 # define DP_SET_POWER_D00x1
@@ -563,6 +580,9 @@
 #define DP_RECEIVER_ALPM_STATUS0x200b  /* eDP 1.4 */
 # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0)
 
+#define DP_DP13_DPCD_REV0x2200
+#define DP_DP13_MAX_LINK_RATE   0x2201
+
 /* DP 1.2 Sideband message defines */
 /* peer device type - DP 1.2a Table 2-92 */
 #define DP_PEER_DEVICE_NONE0x0
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

some minor dal cleanups and example of future patches

[Dave Airlie]

So whenever I look at DAL I find myself trying to figure out what
all of it does, and this leads me to deleting things, so I'm just
sending out some stuff that I cleaned today.

One thing I noticed in passing is the displayport code seems
possibly endian unsafe, it casts bytes read from hardware into
bitfields, this is the best defined behaviour and is generally
a pattern we try to steer away from.

The porting the displayport code to use drm defines is the
sort of example of a cleanup patch that might cause problems
for you in code sharing, but is the exact sort of patch that
has been applied to other drivers in the past.

Dave.

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/8] dc: remove dc hub - this seems unused.

[Dave Airlie]

From: Dave Airlie 

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c   | 27 --
 drivers/gpu/drm/amd/display/dc/dc.h| 17 --
 .../drm/amd/display/dc/dce110/dce110_mem_input.c   |  1 -
 .../gpu/drm/amd/display/dc/dce80/dce80_mem_input.c |  1 -
 drivers/gpu/drm/amd/display/dc/inc/hw/mem_input.h  |  3 ---
 5 files changed, 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 8e1d695..8697d7c 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1828,30 +1828,3 @@ const struct dc_stream_status *dc_stream_get_status(
return >status;
 }
 
-bool dc_init_dchub(struct dc *dc, struct dchub_init_data *dh_data)
-{
-   int i;
-   struct core_dc *core_dc = DC_TO_CORE(dc);
-   struct mem_input *mi = NULL;
-
-   for (i = 0; i < core_dc->res_pool->pipe_count; i++) {
-   if (core_dc->res_pool->mis[i] != NULL) {
-   mi = core_dc->res_pool->mis[i];
-   break;
-   }
-   }
-   if (mi == NULL) {
-   dm_error("no mem_input!\n");
-   return false;
-   }
-
-   if (mi->funcs->mem_input_update_dchub)
-   mi->funcs->mem_input_update_dchub(mi, dh_data);
-   else
-   ASSERT(mi->funcs->mem_input_update_dchub);
-
-
-   return true;
-
-}
-
diff --git a/drivers/gpu/drm/amd/display/dc/dc.h 
b/drivers/gpu/drm/amd/display/dc/dc.h
index 5f60800..376981e 100644
--- a/drivers/gpu/drm/amd/display/dc/dc.h
+++ b/drivers/gpu/drm/amd/display/dc/dc.h
@@ -157,21 +157,6 @@ struct dc {
struct dc_debug debug;
 };
 
-enum frame_buffer_mode {
-   FRAME_BUFFER_MODE_LOCAL_ONLY = 0,
-   FRAME_BUFFER_MODE_ZFB_ONLY,
-   FRAME_BUFFER_MODE_MIXED_ZFB_AND_LOCAL,
-} ;
-
-struct dchub_init_data {
-   bool dchub_initialzied;
-   bool dchub_info_valid;
-   int64_t zfb_phys_addr_base;
-   int64_t zfb_mc_base_addr;
-   uint64_t zfb_size_in_byte;
-   enum frame_buffer_mode fb_mode;
-};
-
 struct dc_init_data {
struct hw_asic_id asic_id;
void *driver; /* ctx */
@@ -192,8 +177,6 @@ struct dc *dc_create(const struct dc_init_data 
*init_params);
 
 void dc_destroy(struct dc **dc);
 
-bool dc_init_dchub(struct dc *dc, struct dchub_init_data *dh_data);
-
 
/***
  * Surface Interfaces
  
**/
diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input.c 
b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input.c
index af9d682..a28bb4d 100644
--- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input.c
+++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_mem_input.c
@@ -396,7 +396,6 @@ static struct mem_input_funcs dce110_mem_input_funcs = {
dce_mem_input_program_surface_config,
.mem_input_is_flip_pending =
dce110_mem_input_is_flip_pending,
-   .mem_input_update_dchub = NULL
 };
 /*/
 /* Constructor, Destructor   */
diff --git a/drivers/gpu/drm/amd/display/dc/dce80/dce80_mem_input.c 
b/drivers/gpu/drm/amd/display/dc/dce80/dce80_mem_input.c
index ebb8df3..704a7ce 100644
--- a/drivers/gpu/drm/amd/display/dc/dce80/dce80_mem_input.c
+++ b/drivers/gpu/drm/amd/display/dc/dce80/dce80_mem_input.c
@@ -54,7 +54,6 @@ static struct mem_input_funcs dce80_mem_input_funcs = {
dce_mem_input_program_surface_config,
.mem_input_is_flip_pending =
dce110_mem_input_is_flip_pending,
-   .mem_input_update_dchub = NULL
 };
 
 /*/
diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mem_input.h 
b/drivers/gpu/drm/amd/display/dc/inc/hw/mem_input.h
index 80566c8..d960db6 100644
--- a/drivers/gpu/drm/amd/display/dc/inc/hw/mem_input.h
+++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mem_input.h
@@ -98,9 +98,6 @@ struct mem_input_funcs {
bool horizontal_mirror);
 
bool (*mem_input_is_flip_pending)(struct mem_input *mem_input);
-
-   void (*mem_input_update_dchub)(struct mem_input *mem_input,
-   struct dchub_init_data *dh_data);
 };
 
 #endif
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 7/8] dal: port to using drm dpcd defines

[Dave Airlie]

From: Dave Airlie 

We only keep one list of these defines in the kernel, so we should use it.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c  |   8 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c  |   2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c   | 144 ++---
 drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c |   2 +-
 drivers/gpu/drm/amd/display/include/dpcd_defs.h| 223 +
 5 files changed, 79 insertions(+), 300 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 47f22d4..657ec0a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -1157,7 +1157,7 @@ static void dpcd_configure_panel_mode(
/*set edp panel mode in receiver*/
core_link_read_dpcd(
link,
-   DPCD_ADDRESS_EDP_CONFIG_SET,
+   DP_EDP_CONFIGURATION_SET,
_config_set.raw,
sizeof(edp_config_set.raw));
 
@@ -1169,7 +1169,7 @@ static void dpcd_configure_panel_mode(
panel_mode_edp;
result = core_link_write_dpcd(
link,
-   DPCD_ADDRESS_EDP_CONFIG_SET,
+   DP_EDP_CONFIGURATION_SET,
_config_set.raw,
sizeof(edp_config_set.raw));
 
@@ -1190,13 +1190,13 @@ static void enable_stream_features(struct pipe_ctx 
*pipe_ctx)
struct core_link *link = stream->sink->link;
union down_spread_ctrl downspread;
 
-   core_link_read_dpcd(link, DPCD_ADDRESS_DOWNSPREAD_CNTL,
+   core_link_read_dpcd(link, DP_DOWNSPREAD_CTRL,
, sizeof(downspread));
 
downspread.bits.IGNORE_MSA_TIMING_PARAM =
(stream->public.ignore_msa_timing_param) ? 1 : 0;
 
-   core_link_write_dpcd(link, DPCD_ADDRESS_DOWNSPREAD_CNTL,
+   core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
, sizeof(downspread));
 }
 
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
index 6379ccf..cd66941 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
@@ -872,7 +872,7 @@ void dal_ddc_service_i2c_query_dp_dual_mode_adaptor(
 
 enum {
DP_SINK_CAP_SIZE =
-   DPCD_ADDRESS_EDP_CONFIG_CAP - DPCD_ADDRESS_DPCD_REV + 1
+   DP_EDP_CONFIGURATION_CAP - DP_DPCD_REV + 1
 };
 
 bool dal_ddc_service_query_ddc_data(
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 2585ec3..d3625c2 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -70,7 +70,7 @@ static void wait_for_training_aux_rd_interval(
 * "DPCD_ADDR_TRAINING_AUX_RD_INTERVAL" register */
core_link_read_dpcd(
link,
-   DPCD_ADDRESS_TRAINING_AUX_RD_INTERVAL,
+   DP_TRAINING_AUX_RD_INTERVAL,
(uint8_t *)_rd_interval,
sizeof(training_rd_interval));
 
@@ -93,14 +93,14 @@ static void dpcd_set_training_pattern(
 {
core_link_write_dpcd(
link,
-   DPCD_ADDRESS_TRAINING_PATTERN_SET,
+   DP_TRAINING_PATTERN_SET,
_pattern.raw,
1);
 
dm_logger_write(link->ctx->logger, LOG_HW_LINK_TRAINING,
"%s\n %x pattern = %x\n",
__func__,
-   DPCD_ADDRESS_TRAINING_PATTERN_SET,
+   DP_TRAINING_PATTERN_SET,
dpcd_pattern.v1_4.TRAINING_PATTERN_SET);
 }
 
@@ -129,19 +129,19 @@ static void dpcd_set_link_settings(
link_set_buffer[0] = rate;
link_set_buffer[1] = lane_count_set.raw;
 
-   core_link_write_dpcd(link, DPCD_ADDRESS_LINK_BW_SET,
+   core_link_write_dpcd(link, DP_LINK_BW_SET,
link_set_buffer, 2);
-   core_link_write_dpcd(link, DPCD_ADDRESS_DOWNSPREAD_CNTL,
+   core_link_write_dpcd(link, DP_DOWNSPREAD_CTRL,
, sizeof(downspread));
 
dm_logger_write(link->ctx->logger, LOG_HW_LINK_TRAINING,
"%s\n %x rate = %x\n %x lane = %x\n %x spread = %x\n",
__func__,
-   DPCD_ADDRESS_LINK_BW_SET,
+   DP_LINK_BW_SET,
lt_settings->link_settings.link_rate,
-   DPCD_ADDRESS_LANE_COUNT_SET,
+   DP_LANE_COUNT_SET,
lt_settings->link_settings.lane_count,
-   DPCD_ADDRESS_DOWNSPREAD_CNTL,
+   DP_DOWNSPREAD_CTRL,

[PATCH 8/8] dal: assign correct enum for edp revision

[Dave Airlie]

From: Dave Airlie 

There are 2 edp enum revisions, no idea why, drop one, and just
assign 1.1 to the default value.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c |  2 +-
 drivers/gpu/drm/amd/display/include/dpcd_defs.h  | 16 
 2 files changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index d3625c2..a4b6a6a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -2129,7 +2129,7 @@ static void retrieve_link_cap(struct core_link *link)
link->dpcd_caps.panel_mode_edp =
edp_config_cap.bits.ALT_SCRAMBLER_RESET;
 
-   link->edp_revision = DPCD_EDP_REVISION_EDP_UNKNOWN;
+   link->edp_revision = EDP_REVISION_11;
 
link->public.test_pattern_enabled = false;
link->public.compliance_test_state.raw = 0;
diff --git a/drivers/gpu/drm/amd/display/include/dpcd_defs.h 
b/drivers/gpu/drm/amd/display/include/dpcd_defs.h
index f0376f4..ba99e6b 100644
--- a/drivers/gpu/drm/amd/display/include/dpcd_defs.h
+++ b/drivers/gpu/drm/amd/display/include/dpcd_defs.h
@@ -144,22 +144,6 @@ enum dpcd_psr_sink_states {
PSR_SINK_STATE_SINK_INTERNAL_ERROR = 7,
 };
 
-/* This enum defines the Panel's eDP revision at DPCD 700h
- * 00h = eDP v1.1 or lower
- * 01h = eDP v1.2
- * 02h = eDP v1.3 (PSR support starts here)
- * 03h = eDP v1.4
- * If unknown revision, treat as eDP v1.1, meaning least functionality set.
- * This enum has values matched to eDP spec, thus values should not change.
- */
-enum dpcd_edp_revision {
-   DPCD_EDP_REVISION_EDP_V1_1 = 0,
-   DPCD_EDP_REVISION_EDP_V1_2 = 1,
-   DPCD_EDP_REVISION_EDP_V1_3 = 2,
-   DPCD_EDP_REVISION_EDP_V1_4 = 3,
-   DPCD_EDP_REVISION_EDP_UNKNOWN = DPCD_EDP_REVISION_EDP_V1_1,
-};
-
 union dpcd_rev {
struct {
uint8_t MINOR:4;
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/8] dal: remove some unused wrappers

[Dave Airlie]

From: Dave Airlie 

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
index 9d51259..9c852a3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
@@ -36,10 +36,6 @@
 #include "amdgpu_dm_types.h"
 #include "amdgpu_pm.h"
 
-#define dm_alloc(size) kzalloc(size, GFP_KERNEL)
-#define dm_realloc(ptr, size) krealloc(ptr, size, GFP_KERNEL)
-#define dm_free(ptr) kfree(ptr)
-
 /**
  * IRQ Interfaces.
  */
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/8] dal: drop get platform info

[Dave Airlie]

From: Dave Airlie 

Signed-off-by: Dave Airlie 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_services.c |  7 -
 drivers/gpu/drm/amd/display/dc/dm_services.h   | 32 --
 2 files changed, 39 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
index 565be05..b842eaf 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
@@ -77,13 +77,6 @@ void dal_isr_release_lock(struct dc_context *ctx)
  * End-of-IRQ Interfaces.
  */
 
-bool dm_get_platform_info(struct dc_context *ctx,
-   struct platform_info_params *params)
-{
-   /*TODO*/
-   return false;
-}
-
 bool dm_write_persistent_data(struct dc_context *ctx,
const struct dc_sink *sink,
const char *module_name,
diff --git a/drivers/gpu/drm/amd/display/dc/dm_services.h 
b/drivers/gpu/drm/amd/display/dc/dm_services.h
index f3f9a40..11a0abf 100644
--- a/drivers/gpu/drm/amd/display/dc/dm_services.h
+++ b/drivers/gpu/drm/amd/display/dc/dm_services.h
@@ -282,38 +282,6 @@ bool dm_pp_get_static_clocks(
 
 /** end of PP interfaces **/
 
-enum platform_method {
-   PM_GET_AVAILABLE_METHODS = 1 << 0,
-   PM_GET_LID_STATE = 1 << 1,
-   PM_GET_EXTENDED_BRIGHNESS_CAPS = 1 << 2
-};
-
-struct platform_info_params {
-   enum platform_method method;
-   void *data;
-};
-
-struct platform_info_brightness_caps {
-   uint8_t ac_level_percentage;
-   uint8_t dc_level_percentage;
-};
-
-struct platform_info_ext_brightness_caps {
-   struct platform_info_brightness_caps basic_caps;
-   struct data_point {
-   uint8_t luminance;
-   uint8_t signal_level;
-   } data_points[99];
-
-   uint8_t data_points_num;
-   uint8_t min_input_signal;
-   uint8_t max_input_signal;
-};
-
-bool dm_get_platform_info(
-   struct dc_context *ctx,
-   struct platform_info_params *params);
-
 struct persistent_data_flag {
bool save_per_link;
bool save_per_edid;
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/8] dal: drop register logger and pid/tgid getters

[Dave Airlie]

From: Dave Airlie 

While I'm sure this is useful I think we should bring it back later.

It's usage of pid/tgid is incorrect, you have to get/put
pid/tgids not store them away.

Signed-off-by: Dave Airlie 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_services.c |  10 --
 drivers/gpu/drm/amd/display/dc/basics/Makefile |   2 +-
 .../drm/amd/display/dc/basics/register_logger.c| 197 -
 drivers/gpu/drm/amd/display/dc/dm_services.h   |  16 --
 .../drm/amd/display/include/dal_register_logger.h  |  42 -
 5 files changed, 1 insertion(+), 266 deletions(-)
 delete mode 100644 drivers/gpu/drm/amd/display/dc/basics/register_logger.c
 delete mode 100644 drivers/gpu/drm/amd/display/include/dal_register_logger.h

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
index 9c852a3..565be05 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
@@ -447,13 +447,3 @@ void dal_notify_setmode_complete(struct dc_context *ctx,
/*TODO*/
 }
 /* End of calls to notification */
-
-long dm_get_pid(void)
-{
-   return current->pid;
-}
-
-long dm_get_tgid(void)
-{
-   return current->tgid;
-}
diff --git a/drivers/gpu/drm/amd/display/dc/basics/Makefile 
b/drivers/gpu/drm/amd/display/dc/basics/Makefile
index a263cad..0658162 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/basics/Makefile
@@ -4,7 +4,7 @@
 # subcomponents.
 
 BASICS = conversion.o fixpt31_32.o fixpt32_32.o grph_object_id.o \
-   logger.o log_helpers.o register_logger.o signal_types.o vector.o
+   logger.o log_helpers.o signal_types.o vector.o
 
 AMD_DAL_BASICS = $(addprefix $(AMDDALPATH)/dc/basics/,$(BASICS))
 
diff --git a/drivers/gpu/drm/amd/display/dc/basics/register_logger.c 
b/drivers/gpu/drm/amd/display/dc/basics/register_logger.c
deleted file mode 100644
index b8d57d9..000
--- a/drivers/gpu/drm/amd/display/dc/basics/register_logger.c
+++ /dev/null
@@ -1,197 +0,0 @@
-/*
- * Copyright 2012-15 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- * Authors: AMD
- *
- */
-
-#include "dm_services.h"
-#include "include/dal_types.h"
-#include "include/logger_interface.h"
-#include "logger.h"
-
-/**
- * Register Logger.
- * A facility to create register R/W logs.
- * Currently used for DAL Test.
- */
-
-/**
- * Private structures
- */
-struct dal_reg_dump_stack_location {
-   const char *current_caller_func;
-   long current_pid;
-   long current_tgid;
-   uint32_t rw_count;/* register access counter for current function. */
-};
-
-/* This the maximum number of nested calls to the 'reg_dump' facility. */
-#define DAL_REG_DUMP_STACK_MAX_SIZE 32
-
-struct dal_reg_dump_stack {
-   int32_t stack_pointer;
-   struct dal_reg_dump_stack_location
-   stack_locations[DAL_REG_DUMP_STACK_MAX_SIZE];
-   uint32_t total_rw_count; /* Total count for *all* functions. */
-};
-
-static struct dal_reg_dump_stack reg_dump_stack = {0};
-
-/**
- * Private functions
- */
-
-/* Check if current process is the one which requested register dump.
- * The reason for the check:
- * mmCRTC_STATUS_FRAME_COUNT is accessed by 
dal_controller_get_vblank_counter().
- * Which runs all the time when at least one display is connected.
- * (Triggered by 

[PATCH 5/8] dal: drop setmode complete notifier

[Dave Airlie]

From: Dave Airlie 

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
index b842eaf..5af27aa 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
@@ -427,16 +427,3 @@ bool dm_pp_get_static_clocks(
 }
 
 / end of power component interfaces /
-
-/* Calls to notification */
-
-void dal_notify_setmode_complete(struct dc_context *ctx,
-   uint32_t h_total,
-   uint32_t v_total,
-   uint32_t h_active,
-   uint32_t v_active,
-   uint32_t pix_clk_in_khz)
-{
-   /*TODO*/
-}
-/* End of calls to notification */
-- 
2.9.3

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Harry Wentland]

On 2016-12-11 03:28 PM, Daniel Vetter wrote:

On Wed, Dec 07, 2016 at 09:02:13PM -0500, Harry Wentland wrote:

We propose to use the Display Core (DC) driver for display support on
AMD's upcoming GPU (referred to by uGPU in the rest of the doc). In order to
avoid a flag day the plan is to only support uGPU initially and transition
to older ASICs gradually.


Bridgeman brought it up a few times that this here was the question - it's
kinda missing a question mark, hard to figure this out ;-). I'd say for


My bad for the missing question mark (imprecise phrasing). On the other 
hand letting this blow over a bit helped get us on the map a bit more 
and allows us to argue the challenges (and benefits) of open source. :)



upstream it doesn't really matter, but imo having both atomic and
non-atomic paths in one driver is one world of hurt and I strongly
recommend against it, at least if feasible. All drivers that switched
switched in one go, the only exception was i915 (it took much longer than
we ever feared, causing lots of pain) and nouveau (which only converted
nv50+, but pre/post-nv50 have always been two almost completely separate
worlds anyway).



You mention the two probably most complex DRM drivers didn't switch in a 
single go...  I imagine amdgpu/DC falls into the same category.


I think one of the problems is making a sudden change with a fully 
validated driver without breaking existing use cases and customers. We 
really should've started DC development in public and probably would do 
that if we had to start anew.



The DC component has received extensive testing within AMD for DCE8, 10, and
11 GPUs and is being prepared for uGPU. Support should be better than
amdgpu's current display support.

 * All of our QA effort is focused on DC
 * All of our CQE effort is focused on DC
 * All of our OEM preloads and custom engagements use DC
 * DC behavior mirrors what we do for other OSes

The new asic utilizes a completely re-designed atom interface, so we cannot
easily leverage much of the existing atom-based code.

We've introduced DC to the community earlier in 2016 and received a fair
amount of feedback. Some of what we've addressed so far are:

 * Self-contain ASIC specific code. We did a bunch of work to pull
   common sequences into dc/dce and leave ASIC specific code in
   separate folders.
 * Started to expose AUX and I2C through generic kernel/drm
   functionality and are mostly using that. Some of that code is still
   needlessly convoluted. This cleanup is in progress.
 * Integrated Dave and Jerome’s work on removing abstraction in bios
   parser.
 * Retire adapter service and asic capability
 * Remove some abstraction in GPIO

Since a lot of our code is shared with pre- and post-silicon validation
suites changes need to be done gradually to prevent breakages due to a major
flag day.  This, coupled with adding support for new asics and lots of new
feature introductions means progress has not been as quick as we would have
liked. We have made a lot of progress none the less.

The remaining concerns that were brought up during the last review that we
are working on addressing:

 * Continue to cleanup and reduce the abstractions in DC where it
   makes sense.
 * Removing duplicate code in I2C and AUX as we transition to using the
   DRM core interfaces.  We can't fully transition until we've helped
   fill in the gaps in the drm core that we need for certain features.
 * Making sure Atomic API support is correct.  Some of the semantics of
   the Atomic API were not particularly clear when we started this,
   however, that is improving a lot as the core drm documentation
   improves.  Getting this code upstream and in the hands of more
   atomic users will further help us identify and rectify any gaps we
   have.


Ok so I guess Dave is typing some more general comments about
demidlayering, let me type some guidelines about atomic. Hopefully this
all materializes itself a bit better into improved upstream docs, but meh.



Excellent writeup. Let us know when/if you want our review for upstream 
docs.


We'll have to really take some time to go over our atomic 
implementation. A couple small comments below with regard to DC.



Step 0: Prep

So atomic is transactional, but it's not validate + rollback or commit,
but duplicate state, validate and then either throw away or commit.
There's a few big reasons for this: a) partial atomic updates - if you
duplicate it's much easier to check that you have all the right locks b)
kfree() is much easier to check for correctness than a rollback code and
c) atomic_check functions are much easier to audit for invalid changes to
persistent state.



There isn't really any rollback. I believe even in our other drivers 
we've abandoned the rollback approach years ago because it doesn't 
really work on modern HW. Any rollback cases you might find in DC should 
really only be for catastrophic errors (read: something went horribly 
wrong... read: 

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Harry Wentland]

On 2016-12-12 02:22 AM, Daniel Vetter wrote:

On Wed, Dec 07, 2016 at 09:02:13PM -0500, Harry Wentland wrote:

Current version of DC:

 * 
https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7

Once Alex pulls in the latest patches:

 * 
https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7


One more: That 4.7 here is going to be unbelievable amounts of pain for
you. Yes it's a totally sensible idea to just freeze your baseline kernel
because then linux looks a lot more like Windows where the driver abi is
frozen. But it makes following upstream entirely impossible, because
rebasing is always a pain and hence postponed. Which means you can't just
use the latest stuff in upstream drm, which means collaboration with
others and sharing bugfixes in core is a lot more pain, which then means
you do more than necessary in your own code and results in HALs like DAL,
perpetuating the entire mess.

So I think you don't just need to demidlayer DAL/DC, you also need to
demidlayer your development process. In our experience here at Intel that
needs continuous integration testing (in drm-tip), because even 1 month of
not resyncing with drm-next is sometimes way too long. See e.g. the
controlD regression we just had. And DAL is stuck on a 1 year old kernel,
so pretty much only of historical significance and otherwise dead code.

And then for any stuff which isn't upstream yet (like your internal
enabling, or DAL here, or our own internal enabling) you need continuous
rebasing When we started doing this years ago it was still
manually, but we still rebased like every few days to keep the pain down
and adjust continuously to upstream evolution. But then going to a
continous rebase bot that sends you mail when something goes wrong was
again a massive improvement.



I think we've seen that pain already but haven't quite realized how much 
of it is due to a mismatch in kernel trees. We're trying to move onto a 
tree following drm-next much more closely. I'd love to help automate 
some of that (time permitting). Would the drm-misc scripts be of any use 
with that? I only had a very cursory glance at those.



I guess in the end Conway's law that your software architecture
necessarily reflects how you organize your teams applies again. Fix your
process and it'll become glaringly obvious to everyone involved that
DC-the-design as-is is entirely unworkeable and how it needs to be fixed.

From my own experience over the past few years: Doing that is a fun
journey ;-)



Absolutely. We're only at the start of this but have learned a lot from 
the community (maybe others in the DC team disagree with me somewhat).


Not sure if I fully agree that this means that DC-the-design-as-is will 
become apparent as unworkable... There are definitely pieces to be 
cleaned here and lessons learned from the DRM community but on the other 
hand we feel there are some good reasons behind our approach that we'd 
like to share with the community (some of which I'm learning myself).


Harry


Cheers, Daniel


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] amdgpu: get maximum and used UVD handles

[arindam . nath]

From: Arindam Nath 

User might want to query the maximum number of UVD
instances supported by firmware. In addition to that,
if there are multiple applications using UVD handles
at the same time, he might also want to query the
currently used number of handles.

For this we add a new query AMDGPU_INFO_NUM_HANDLES
and a new struct drm_amdgpu_info_num_handles to
get these values.

Signed-off-by: Arindam Nath 
Reviewed-by: Christian König 
---
 include/drm/amdgpu_drm.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
index d8f2497..89fda6b 100644
--- a/include/drm/amdgpu_drm.h
+++ b/include/drm/amdgpu_drm.h
@@ -483,6 +483,8 @@ struct drm_amdgpu_cs_chunk_data {
 #define AMDGPU_INFO_DEV_INFO   0x16
 /* visible vram usage */
 #define AMDGPU_INFO_VIS_VRAM_USAGE 0x17
+/* Query UVD handles */
+#define AMDGPU_INFO_NUM_HANDLES0x1C
 
 #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
 #define AMDGPU_INFO_MMR_SE_INDEX_MASK  0xff
@@ -641,6 +643,13 @@ struct drm_amdgpu_info_hw_ip {
uint32_t  _pad;
 };
 
+struct drm_amdgpu_info_num_handles {
+   /** Max handles as supported by firmware for UVD */
+   uint32_t  uvd_max_handles;
+   /** Handles currently in use for UVD */
+   uint32_t  uvd_used_handles;
+};
+
 /*
  * Supported GPU families
  */
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/1] drm/amd/amdgpu: get maximum and used UVD handles (v3)

[arindam . nath]

From: Arindam Nath 

Change History
--

v3: changes suggested by Christian
- Add a check for UVD IP block using AMDGPU_HW_IP_UVD
  query type.
- Add a check for asic_type to be less than
  CHIP_POLARIS10 since starting Polaris, we support
  unlimited UVD instances.
- Add kerneldoc style comment for
  amdgpu_uvd_used_handles().

v2: as suggested by Christian
- Add a new query AMDGPU_INFO_NUM_HANDLES
- Create a helper function to return the number
  of currently used UVD handles.
- Modify the logic to count the number of used
  UVD handles since handles can be freed in
  non-linear fashion.

v1:
- User might want to query the maximum number of UVD
  instances supported by firmware. In addition to that,
  if there are multiple applications using UVD handles
  at the same time, he might also want to query the
  currently used number of handles.

  For this we add two variables max_handles and
  used_handles inside drm_amdgpu_info_hw_ip. So now
  an application (or libdrm) can use AMDGPU_INFO IOCTL
  with AMDGPU_INFO_HW_IP_INFO query type to get these
  values.

Signed-off-by: Arindam Nath 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 21 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 25 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h |  1 +
 include/uapi/drm/amdgpu_drm.h   |  9 +
 4 files changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 174eb59..3273d8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -570,6 +570,27 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
return -EINVAL;
}
}
+   case AMDGPU_INFO_NUM_HANDLES: {
+   struct drm_amdgpu_info_num_handles handle;
+
+   switch (info->query_hw_ip.type) {
+   case AMDGPU_HW_IP_UVD:
+   /* Starting Polaris, we support unlimited UVD handles */
+   if (adev->asic_type < CHIP_POLARIS10) {
+   handle.uvd_max_handles = adev->uvd.max_handles;
+   handle.uvd_used_handles = 
amdgpu_uvd_used_handles(adev);
+
+   return copy_to_user(out, ,
+   min((size_t)size, sizeof(handle))) ? 
-EFAULT : 0;
+   } else {
+   return -EINVAL;
+   }
+
+   break;
+   default:
+   return -EINVAL;
+   }
+   }
default:
DRM_DEBUG_KMS("Invalid request %d\n", info->query);
return -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index a8816ba..02187fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1173,3 +1173,28 @@ int amdgpu_uvd_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
 error:
return r;
 }
+
+/**
+ * amdgpu_uvd_used_handles - returns used UVD handles
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Returns the number of UVD handles in use
+ */
+uint32_t amdgpu_uvd_used_handles(struct amdgpu_device *adev)
+{
+   unsigned i;
+   uint32_t used_handles = 0;
+
+   for (i = 0; i < adev->uvd.max_handles; ++i) {
+   /*
+* Handles can be freed in any order, and not
+* necessarily linear. So we need to count
+* all non-zero handles.
+*/
+   if (atomic_read(>uvd.handles[i]))
+   used_handles++;
+   }
+
+   return used_handles;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
index c850009..0fee861 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.h
@@ -38,5 +38,6 @@ int amdgpu_uvd_ring_parse_cs(struct amdgpu_cs_parser *parser, 
uint32_t ib_idx);
 void amdgpu_uvd_ring_begin_use(struct amdgpu_ring *ring);
 void amdgpu_uvd_ring_end_use(struct amdgpu_ring *ring);
 int amdgpu_uvd_ring_test_ib(struct amdgpu_ring *ring, long timeout);
+uint32_t amdgpu_uvd_used_handles(struct amdgpu_device *adev);
 
 #endif
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index bea9875..2cf8df8 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -530,6 +530,8 @@ struct drm_amdgpu_cs_chunk_data {
#define AMDGPU_INFO_VBIOS_SIZE  0x1
/* Subquery id: Query vbios image */
#define AMDGPU_INFO_VBIOS_IMAGE 0x2
+/* Query UVD handles */
+#define AMDGPU_INFO_NUM_HANDLES0x1C
 
 #define AMDGPU_INFO_MMR_SE_INDEX_SHIFT 0
 #define 

Re: Raciness with page table shadows being swapped out

[Christian König]

Am 12.12.2016 um 16:20 schrieb Nicolai Hähnle:

Hi all,

I just sent out two patches that hopefully make the kernel module more 
robust in the face of page table shadows being swapped out.


However, even with those patches, I can still fairly reliably 
reproduce crashes with a backtrace of the shape


amdgpu_cs_ioctl
 -> amdgpu_vm_update_page_directory
 -> amdgpu_ttm_bind
 -> amdgpu_gtt_mgr_alloc

The plausible reason for these crashes is that nothing seems to 
prevent the shadow BOs from being moved between the calls to 
amdgpu_cs_validate in amdgpu_cs_parser_bos and the calls to 
amdgpu_ttm_bind.


The shadow BOs use the same reservation object than the real BOs. So as 
long as the real BOs can't be evicted the shadows can't be evicted either.




The attached patch has fixed these crashes for me so far, but it's 
very heavy-handed: it collects all page table shadows and the page 
directory shadow and adds them all to the reservations for the callers 
of amdgpu_vm_update_page_directory.


That is most likely just a timing change, cause the shadows should end 
up in the duplicates list anyway. So the patch shouldn't have any effect.




I feel like there should be a better way. In part, I wonder why the 
shadows are needed in the first place. I vaguely recall the 
discussions about GPU reset and such, but I don't remember why the 
page tables can't just be rebuilt in some other way.


It's just the simplest and fastest way to keep a copy of the page tables 
around.


The problem with rebuilding the page tables from the mappings is that 
the housekeeping structures already have the future state when a reset 
happens, not the state we need to rebuild the tables.


We could obviously change the housekeeping a bit to keep both states, 
but that would complicate mapping and unmapping of BOs significantly.


Regards,
Christian.



Cheers,
Nicolai


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [RFC] Using DC in amdgpu for upcoming GPU

[Deucher, Alexander]

> -Original Message-
> From: Luke A. Guest [mailto:lagu...@archeia.com]
> Sent: Monday, December 12, 2016 11:17 AM
> To: Deucher, Alexander; 'Daniel Vetter'; Bridgman, John
> Cc: Grodzovsky, Andrey; Cheng, Tony; amd-gfx@lists.freedesktop.org; dri-
> de...@lists.freedesktop.org
> Subject: Re: [RFC] Using DC in amdgpu for upcoming GPU
> 
> On 12/12/16 15:28, Deucher, Alexander wrote:
> >> -Original Message-
> >> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On
> Behalf
> >> Of Daniel Vetter
> >> Sent: Monday, December 12, 2016 4:27 AM
> >> To: Bridgman, John
> >> Cc: Grodzovsky, Andrey; Cheng, Tony; dri-de...@lists.freedesktop.org;
> amd-
> >> g...@lists.freedesktop.org; Daniel Vetter; Deucher, Alexander; Wentland,
> >> Harry
> >> Subject: Re: [RFC] Using DC in amdgpu for upcoming GPU
> >>
> >> On Mon, Dec 12, 2016 at 07:54:54AM +, Bridgman, John wrote:
> >>> Yep, good point. We have tended to stay a bit behind bleeding edge
> >> because our primary tasks so far have been:
> >>>
> >>> 1. Support enterprise distros (with old kernels) via the hybrid driver
> >>> (AMDGPU-PRO), where the closer to upstream we get the more of a
> gap
> >> we
> >>> have to paper over with KCL code
> >> Hm, I thought resonable enterprise distros roll their drm core forward to
> >> the very latest upstream fairly often, so it shouldn't be too bad? Fixing
> >> this completely requires that you upstream your pre-production hw
> support
> >> early enough that by the time it ships its the backport is already in a
> >> realeased enterprise distro upgrade. But then adding bugfixes on top
> >> should be doable.
> > The issue is we need DAL/DC for enterprise distros and OEM preloads and,
> for workstation customers, we need some additional patches that aren't
> upstream yet because they we don’t have an open source user for them yet.
> This gets much easier once we get OCL and VK open sourced.  As for new asic
> support, unfortunately, they do not often align well with enterprise distros 
> at
> least for dGPUs (APUs are usually easier since the cycles are longer, dGPUs
> cycles are very fast).  The other problem with dGPUs is that we often can't
> release support for new hw or feature too much earlier than launch due to
> the very competitive dGPU environment in gaming and workstation.
> >
> >
> Apologies for spamming, but I didn't send this to all.
> 
> What Daniel said is something I've said to you before, especially
> regarding libdrm. You keep mentioning these patches you need, but tbh,
> there's no reason why these patches cannot be in patchwork so people can
> use them. I've asked for this for months and the response was,
> "shouldn't be a problem, but I won't get to it this week," months later,
> still not there.

The kernel side is public.  The dkms packages have the full source tree.  As I 
said before, we plan to make this all public, but just haven't had the time (as 
this thread shows, we've got a lot of other higher priority things on our 
plate).  Even when we do, it doesn’t change the fact that the patches can't go 
upstream at the moment so it doesn't fix the situation Daniel was talking about 
anyway.  Distro's generally don't take code that is not upstream yet.  While we 
only validate the dkms packages on the enterprise distros, they should be 
adaptable to other kernels.

Alex

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Luke A. Guest]

On 12/12/16 15:28, Deucher, Alexander wrote:
>> -Original Message-
>> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
>> Of Daniel Vetter
>> Sent: Monday, December 12, 2016 4:27 AM
>> To: Bridgman, John
>> Cc: Grodzovsky, Andrey; Cheng, Tony; dri-de...@lists.freedesktop.org; amd-
>> g...@lists.freedesktop.org; Daniel Vetter; Deucher, Alexander; Wentland,
>> Harry
>> Subject: Re: [RFC] Using DC in amdgpu for upcoming GPU
>>
>> On Mon, Dec 12, 2016 at 07:54:54AM +, Bridgman, John wrote:
>>> Yep, good point. We have tended to stay a bit behind bleeding edge
>> because our primary tasks so far have been:
>>>
>>> 1. Support enterprise distros (with old kernels) via the hybrid driver
>>> (AMDGPU-PRO), where the closer to upstream we get the more of a gap
>> we
>>> have to paper over with KCL code
>> Hm, I thought resonable enterprise distros roll their drm core forward to
>> the very latest upstream fairly often, so it shouldn't be too bad? Fixing
>> this completely requires that you upstream your pre-production hw support
>> early enough that by the time it ships its the backport is already in a
>> realeased enterprise distro upgrade. But then adding bugfixes on top
>> should be doable.
> The issue is we need DAL/DC for enterprise distros and OEM preloads and, for 
> workstation customers, we need some additional patches that aren't upstream 
> yet because they we don’t have an open source user for them yet.  This gets 
> much easier once we get OCL and VK open sourced.  As for new asic support, 
> unfortunately, they do not often align well with enterprise distros at least 
> for dGPUs (APUs are usually easier since the cycles are longer, dGPUs cycles 
> are very fast).  The other problem with dGPUs is that we often can't release 
> support for new hw or feature too much earlier than launch due to the very 
> competitive dGPU environment in gaming and workstation.
>
>
Apologies for spamming, but I didn't send this to all.

What Daniel said is something I've said to you before, especially
regarding libdrm. You keep mentioning these patches you need, but tbh,
there's no reason why these patches cannot be in patchwork so people can
use them. I've asked for this for months and the response was,
"shouldn't be a problem, but I won't get to it this week," months later,
still not there.

Please just get your stuff public so the people who aren't on enterprise
and ancient OSes can upgrade their systems. This would enable me to test
amdgpu-pro and latest Mesa/LLVM alongside each other for Gentoo without
having to replace a source built libdrm with your ancient one.

Luke.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Luke A. Guest]

On 12/12/16 15:28, Deucher, Alexander wrote:
>> -Original Message-
>> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
>> Of Daniel Vetter
>> Sent: Monday, December 12, 2016 4:27 AM
>> To: Bridgman, John
>> Cc: Grodzovsky, Andrey; Cheng, Tony; dri-de...@lists.freedesktop.org; amd-
>> g...@lists.freedesktop.org; Daniel Vetter; Deucher, Alexander; Wentland,
>> Harry
>> Subject: Re: [RFC] Using DC in amdgpu for upcoming GPU
>>
>> On Mon, Dec 12, 2016 at 07:54:54AM +, Bridgman, John wrote:
>>> Yep, good point. We have tended to stay a bit behind bleeding edge
>> because our primary tasks so far have been:
>>>
>>> 1. Support enterprise distros (with old kernels) via the hybrid driver
>>> (AMDGPU-PRO), where the closer to upstream we get the more of a gap
>> we
>>> have to paper over with KCL code
>> Hm, I thought resonable enterprise distros roll their drm core forward to
>> the very latest upstream fairly often, so it shouldn't be too bad? Fixing
>> this completely requires that you upstream your pre-production hw support
>> early enough that by the time it ships its the backport is already in a
>> realeased enterprise distro upgrade. But then adding bugfixes on top
>> should be doable.
> The issue is we need DAL/DC for enterprise distros and OEM preloads and, for 
> workstation customers, we need some additional patches that aren't upstream 
> yet because they we don’t have an open source user for them yet.  This gets 
> much easier once we get OCL and VK open sourced.  As for new asic support, 
> unfortunately, they do not often align well with enterprise distros at least 
> for dGPUs (APUs are usually easier since the cycles are longer, dGPUs cycles 
> are very fast).  The other problem with dGPUs is that we often can't release 
> support for new hw or feature too much earlier than launch due to the very 
> competitive dGPU environment in gaming and workstation.
>

What Daniel said is something I've said to you before, especially
regarding libdrm. You keep mentioning these patches you need, but tbh,
there's no reason why these patches cannot be in patchwork so people can
use them. I've asked for this for months and the response was,
"shouldn't be a problem, but I won't get to it this week," months later,
still not there.

Please just get your stuff public so the people who aren't on enterprise
and ancient OSes can upgrade their systems. This would enable me to test
amdgpu-pro and latest Mesa/LLVM alongside each other for Gentoo without
having to replace a source built libdrm with your ancient one.

Luke.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Raciness with page table shadows being swapped out

[Nicolai Hähnle]

Hi all,

I just sent out two patches that hopefully make the kernel module more 
robust in the face of page table shadows being swapped out.


However, even with those patches, I can still fairly reliably reproduce 
crashes with a backtrace of the shape


amdgpu_cs_ioctl
 -> amdgpu_vm_update_page_directory
 -> amdgpu_ttm_bind
 -> amdgpu_gtt_mgr_alloc

The plausible reason for these crashes is that nothing seems to prevent 
the shadow BOs from being moved between the calls to amdgpu_cs_validate 
in amdgpu_cs_parser_bos and the calls to amdgpu_ttm_bind.


The attached patch has fixed these crashes for me so far, but it's very 
heavy-handed: it collects all page table shadows and the page directory 
shadow and adds them all to the reservations for the callers of 
amdgpu_vm_update_page_directory.


I feel like there should be a better way. In part, I wonder why the 
shadows are needed in the first place. I vaguely recall the discussions 
about GPU reset and such, but I don't remember why the page tables can't 
just be rebuilt in some other way.


Cheers,
Nicolai
>From 70706d2283773f19b42312bc34e36c4717a9ce62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= 
Date: Mon, 12 Dec 2016 15:02:27 +0100
Subject: [PATCH] drm/amd/amdgpu: reserve shadows of page directory and tables
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Nicolai Hähnle 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  9 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 45 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 +++
 5 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f53e52f..7a84648 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -916,20 +916,21 @@ struct amdgpu_cs_parser {
 	unsigned		nchunks;
 	struct amdgpu_cs_chunk	*chunks;
 
 	/* scheduler job object */
 	struct amdgpu_job	*job;
 
 	/* buffer objects */
 	struct ww_acquire_ctx		ticket;
 	struct amdgpu_bo_list		*bo_list;
 	struct amdgpu_bo_list_entry	vm_pd;
+	struct amdgpu_bo_list_entry	*vm_pd_pt_shadows;
 	struct list_head		validated;
 	struct dma_fence		*fence;
 	uint64_t			bytes_moved_threshold;
 	uint64_t			bytes_moved;
 	struct amdgpu_bo_list_entry	*evictable;
 
 	/* user fence */
 	struct amdgpu_bo_list_entry	uf_entry;
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 985f842..eecf2eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -510,20 +510,26 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	p->bo_list = amdgpu_bo_list_get(fpriv, cs->in.bo_list_handle);
 	if (p->bo_list) {
 		need_mmap_lock = p->bo_list->first_userptr !=
 			p->bo_list->num_entries;
 		amdgpu_bo_list_get_list(p->bo_list, >validated);
 	}
 
 	INIT_LIST_HEAD();
 	amdgpu_vm_get_pd_bo(>vm, >validated, >vm_pd);
 
+	r = amdgpu_vm_get_pd_pt_shadows(>vm, >validated, >vm_pd_pt_shadows);
+	if (r) {
+		DRM_ERROR("amdgpu_vm_get_pd_pt_shadows failed.\n");
+		goto error_release_list;
+	}
+
 	if (p->uf_entry.robj)
 		list_add(>uf_entry.tv.head, >validated);
 
 	if (need_mmap_lock)
 		down_read(>mm->mmap_sem);
 
 	while (1) {
 		struct list_head need_pages;
 		unsigned i;
 
@@ -673,20 +679,21 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 	if (r) {
 		amdgpu_vm_move_pt_bos_in_lru(p->adev, >vm);
 		ttm_eu_backoff_reservation(>ticket, >validated);
 	}
 
 error_free_pages:
 
 	if (need_mmap_lock)
 		up_read(>mm->mmap_sem);
 
+error_release_list:
 	if (p->bo_list) {
 		for (i = p->bo_list->first_userptr;
 		 i < p->bo_list->num_entries; ++i) {
 			e = >bo_list->array[i];
 
 			if (!e->user_pages)
 continue;
 
 			release_pages(e->user_pages,
   e->robj->tbo.ttm->num_pages,
@@ -736,20 +743,22 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, bo
 		ttm_eu_backoff_reservation(>ticket,
 	   >validated);
 	}
 	dma_fence_put(parser->fence);
 
 	if (parser->ctx)
 		amdgpu_ctx_put(parser->ctx);
 	if (parser->bo_list)
 		amdgpu_bo_list_put(parser->bo_list);
 
+	drm_free_large(parser->vm_pd_pt_shadows);
+
 	for (i = 0; i < parser->nchunks; i++)
 		drm_free_large(parser->chunks[i].kdata);
 	kfree(parser->chunks);
 	if (parser->job)
 		amdgpu_job_free(parser->job);
 	amdgpu_bo_unref(>uf_entry.robj);
 }
 
 static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p,
    struct amdgpu_vm *vm)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index ffb3e70..379ccde 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -495,39 +495,44 @@ static int 

Re: [RFC] Using DC in amdgpu for upcoming GPU

[Daniel Vetter]

On Wed, Dec 07, 2016 at 09:02:13PM -0500, Harry Wentland wrote:
> Current version of DC:
> 
>  * 
> https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7
> 
> Once Alex pulls in the latest patches:
> 
>  * 
> https://cgit.freedesktop.org/~agd5f/linux/tree/drivers/gpu/drm/amd/display?h=amd-staging-4.7

One more: That 4.7 here is going to be unbelievable amounts of pain for
you. Yes it's a totally sensible idea to just freeze your baseline kernel
because then linux looks a lot more like Windows where the driver abi is
frozen. But it makes following upstream entirely impossible, because
rebasing is always a pain and hence postponed. Which means you can't just
use the latest stuff in upstream drm, which means collaboration with
others and sharing bugfixes in core is a lot more pain, which then means
you do more than necessary in your own code and results in HALs like DAL,
perpetuating the entire mess.

So I think you don't just need to demidlayer DAL/DC, you also need to
demidlayer your development process. In our experience here at Intel that
needs continuous integration testing (in drm-tip), because even 1 month of
not resyncing with drm-next is sometimes way too long. See e.g. the
controlD regression we just had. And DAL is stuck on a 1 year old kernel,
so pretty much only of historical significance and otherwise dead code.

And then for any stuff which isn't upstream yet (like your internal
enabling, or DAL here, or our own internal enabling) you need continuous
rebasing When we started doing this years ago it was still
manually, but we still rebased like every few days to keep the pain down
and adjust continuously to upstream evolution. But then going to a
continous rebase bot that sends you mail when something goes wrong was
again a massive improvement.

I guess in the end Conway's law that your software architecture
necessarily reflects how you organize your teams applies again. Fix your
process and it'll become glaringly obvious to everyone involved that
DC-the-design as-is is entirely unworkeable and how it needs to be fixed.

From my own experience over the past few years: Doing that is a fun
journey ;-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx