Re: [PATCH] drm/amd/display: Don't load DMCU for Raven 1 (v2)

2019-05-30 Thread Samantha McVey
Alex,

Are any of these non-booting systems laptops? If all the laptops don't have 
this issue, is 
there a way we can detect we are on mobile and load the DMCU? Seeing as ABM and 
PSR 
are both practically only used on mobile, maybe we can check for that. This way 
we only 
enable it on systems that actually need the features. I am guessing the number 
of Raven 
Ridge laptops is much much smaller than the number of motherboards which can 
potentially support Raven Ridge.

On vrijdag 24 mei 2019 18:49:27 CEST you wrote:
> On Fri, May 24, 2019 at 12:32 PM Mike Lothian  wrote:
> > I realise you don't want to enable this as it's breaking some people's
> > systems, but could we add a new boot parameter to force it for working
> > systems? Or check against a black list maybe?
> 
> We could probably add a whitelist.  I'm not sure what the best way to
> id the working systems are though.
> 
> Alex
> 
> > On Fri, 24 May 2019 at 17:20, Alex Deucher  wrote:
> > > On Fri, May 24, 2019 at 12:09 PM Mike Lothian  
> > > wrote:
> > > > Hi
> > > > 
> > > > Curious to know what this means for folk that have newer Raven1 boards
> > > > that didn't have issues loading the firmware
> > > 
> > > You won't get ABM I think.  ABM is the automatic backlight management.
> > > 
> > > Alex
> > > 
> > > > Cheers
> > > > 
> > > > Mike
> > > > 
> > > > On Fri, 24 May 2019 at 16:34, Alex Deucher  
> > > > wrote:
> > > > > From: Harry Wentland 
> > > > > 
> > > > > [WHY]
> > > > > Some early Raven boards had a bad SBIOS that doesn't play nicely
> > > > > with
> > > > > the DMCU FW. We thought the issues were fixed by ignoring errors on
> > > > > DMCU
> > > > > load but that doesn't seem to be the case. We've still seen reports
> > > > > of
> > > > > users unable to boot their systems at all.
> > > > > 
> > > > > [HOW]
> > > > > Disable DMCU load on Raven 1. Only load it for Raven 2 and Picasso.
> > > > > 
> > > > > v2: Fix ifdef (Alex)
> > > > > 
> > > > > Signed-off-by: Harry Wentland 
> > > > > Reviewed-by: Nicholas Kazlauskas 
> > > > > Signed-off-by: Alex Deucher 
> > > > > Cc: stable at vger.kernel.org
> > > > > ---
> > > > > 
> > > > >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 ++--
> > > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index
> > > > > 995f9df66142..bcb1a93c0b4c 100644
> > > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > > > > @@ -29,6 +29,7 @@
> > > > > 
> > > > >  #include "dm_services_types.h"
> > > > >  #include "dc.h"
> > > > >  #include "dc/inc/core_types.h"
> > > > > 
> > > > > +#include "dal_asic_id.h"
> > > > > 
> > > > >  #include "vid.h"
> > > > >  #include "amdgpu.h"
> > > > > 
> > > > > @@ -640,7 +641,7 @@ static void amdgpu_dm_fini(struct amdgpu_device
> > > > > *adev)
> > > > > 
> > > > >  static int load_dmcu_fw(struct amdgpu_device *adev)
> > > > >  {
> > > > > 
> > > > > -   const char *fw_name_dmcu;
> > > > > +   const char *fw_name_dmcu = NULL;
> > > > > 
> > > > > int r;
> > > > > const struct dmcu_firmware_header_v1_0 *hdr;
> > > > > 
> > > > > @@ -663,7 +664,14 @@ static int load_dmcu_fw(struct amdgpu_device
> > > > > *adev)
> > > > > 
> > > > > case CHIP_VEGA20:
> > > > > return 0;
> > > > > 
> > > > > case CHIP_RAVEN:
> > > > > -   fw_name_dmcu = FIRMWARE_RAVEN_DMCU;
> > > > > +#if defined(CONFIG_DRM_AMD_DC_DCN1_01)
> > > > > +   if (ASICREV_IS_PICASSO(adev->external_rev_id))
> > > > > +   fw_name_dmcu = FIRMWARE_RAVEN_DMCU;
> > > > > +   else if (ASICREV_IS_RAVEN2(adev->external_rev_id))
> > > > > +   fw_name_dmcu = FIRMWARE_RAVEN_DMCU;
> > > > > +   else
> > > > > +#endif
> > > > > +   return 0;
> > > > > 
> > > > > break;
> > > > > 
> > > > > default:
> > > > > DRM_ERROR("Unsupported ASIC type: 0x%X\n",
> > > > > adev->asic_type);
> > > > > ___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v3 2/2] drm/amd: Add abm level drm property

2018-11-19 Thread Samantha McVey

David,

Your patch works great. If you are not aware, the ABM does not work 
anymore after the display turns off and back on again. Setting ABM level 
to 0 and back to 4 makes it start working again though. This happens 
when my display turns off automatically, but you can replicate this by 
running `xset dpms force off`.


On 15-11-18 07:20, David.Francis at amd.com (David Francis) wrote:

Adaptive Backlight Management (ABM) is a feature
that reduces backlight level to save power, while
increasing pixel contrast and pixel luminance
to maintain readability and image quality.

ABM will adjust in response to the
pixel luminance of the displayed content.

ABM is made available as a drm property on eDP
monitors called "abm level", which ranges from 0 to 4.
When this property is set to 0, ABM is off.  Levels 1
to 4 represent different ranges of backlight reduction.
At higher levels both the backlight reduction and pixel
adjustment will be greater.

ABM requires DMCU firmware, which is currently available for
Raven ASICs only.  If the feature does not work, please
ensure your firmware is up to date.

v2:
Fix commit message, only attach property if DMCU loaded
v3:
Storre ABM level in crtc state to accommodate dc

Signed-off-by: David Francis 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  5 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  2 ++
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 35 ---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 ++
  drivers/gpu/drm/amd/display/dc/core/dc.c  | 11 +-
  drivers/gpu/drm/amd/display/dc/dc.h   |  1 +
  6 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 7d6a36bca9dd..ced8cefa223b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -637,6 +637,11 @@ int amdgpu_display_modeset_create_props(struct 
amdgpu_device *adev)
 "freesync_capable");
if (!adev->mode_info.freesync_capable_property)
return -ENOMEM;
+   adev->mode_info.abm_level_property =
+   drm_property_create_range(adev->ddev, 0,
+   "abm level", 0, 4);
+   if (!adev->mode_info.abm_level_property)
+   return -ENOMEM;
}
  
  	return 0;

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 1627dd3413c7..2938635c0fc1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -342,6 +342,8 @@ struct amdgpu_mode_info {
struct drm_property *freesync_property;
/* it is used to know about display capability of freesync mode */
struct drm_property *freesync_capable_property;
+   /* Adaptive Backlight Modulation (power feature) */
+   struct drm_property *abm_level_property;
/* hardcoded DFP edid from BIOS */
struct edid *bios_hardcoded_edid;
int bios_hardcoded_edid_size;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index f71febb4210d..090a602f3014 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2920,6 +2920,7 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
state->adjust = cur->adjust;
state->vrr_infopacket = cur->vrr_infopacket;
state->freesync_enabled = cur->freesync_enabled;
+   state->abm_level = cur->abm_level;
  
  	/* TODO Duplicate dc_stream after objects are stream object is flattened */
  
@@ -3038,6 +3039,9 @@ int amdgpu_dm_connector_atomic_set_property(struct drm_connector *connector,

} else if (property == adev->mode_info.freesync_capable_property) {
dm_new_state->freesync_capable = val;
ret = 0;
+   } else if (property == adev->mode_info.abm_level_property) {
+   dm_new_state->abm_level = val;
+   ret = 0;
}
  
  	return ret;

@@ -3086,7 +3090,11 @@ int amdgpu_dm_connector_atomic_get_property(struct 
drm_connector *connector,
} else if (property == adev->mode_info.freesync_capable_property) {
*val = dm_state->freesync_capable;
ret = 0;
+   } else if (property == adev->mode_info.abm_level_property) {
+   *val = dm_state->abm_level;
+   ret = 0;
}
+
return ret;
  }
  
@@ -3151,6 +3159,7 @@ amdgpu_dm_connector_atomic_duplicate_state(struct drm_connector *connector)
  
  	new_state->freesync_capable = state->freesync_capable;

new_state->freesync_enable = state->freesync_enable;
+   new_state->abm_level = state->abm_level;
  
  	return _state->base;

  }
@@ -3904,6 +3913,12 @@ void