| Message ID | 20230322160513.1438881-1-Rodrigo.Siqueira@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/display: Add missing OLED Vesa brightnesses definitions | expand |
On 3/22/23 12:05, Rodrigo Siqueira wrote: > Cc: Anthony Koo <anthony.koo@amd.com> > Cc: Iswara Negulendran <iswara.nagulendran@amd.com> > Cc: Felipe Clark <felipe.clark@amd.com> > Cc: Harry Wentland <Harry.Wentland@amd.com> > Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > include/drm/display/drm_dp.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 632376c291db..d30a9b2f450c 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP (1 << 4) > > #define DP_EDP_GENERAL_CAP_2 0x703 > # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0)
[AMD Official Use Only - General] Hello Rodrigo and Harry, I would like to propose some changes to keep this patch consistent with the naming scheme and general organization of the drm_dp.h file. #define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 It would be better to use the (1<<7) representation for this bit to follow the pattern established by the other defines in the file. Also, a more generic name for this macro would be DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE. # define DP_EDP_OLED_VESA_CAP (1 << 4) A more generic name for this macro would be DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE In terms of the file structure, DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE should appear underneath the definition of DP_EDP_BACKLIGHT_MODE_SET_REGISTER. Similarly, DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE should appear underneath the definition of DP_EDP_GENERAL_CAP_2 For a complete definition of the millinit based brightness control specification the following should also be added: #define DP_EDP_PANEL_TARGET_LUMINANCE_VALUE 0x734 Here is a suggested pseudo-patch with all these changes: #define DP_EDP_GENERAL_CAP_2 0x703 # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0) +# define DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE (1<<7) # define DP_EDP_DYNAMIC_BACKLIGHT_ENABLE (1 << 4) # define DP_EDP_REGIONAL_BACKLIGHT_ENABLE (1 << 5) # define DP_EDP_UPDATE_REGION_BRIGHTNESS (1 << 6) /* eDP 1.4 */ +# define DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE (1<<7) #define DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET 0x732 #define DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET 0x733 +#define DP_EDP_PANEL_TARGET_LUMINANCE_VALUE 0x734 Thank you, Felipe -----Original Message----- From: Wentland, Harry <Harry.Wentland@amd.com> Sent: Wednesday, March 22, 2023 2:01 PM To: Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; airlied@gmail.com; daniel@ffwll.ch Cc: amd-gfx@lists.freedesktop.org; Koo, Anthony <Anthony.Koo@amd.com>; Nagulendran, Iswara <Iswara.Nagulendran@amd.com>; Clark, Felipe <Felipe.Clark@amd.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/display: Add missing OLED Vesa brightnesses definitions On 3/22/23 12:05, Rodrigo Siqueira wrote: > Cc: Anthony Koo <anthony.koo@amd.com> > Cc: Iswara Negulendran <iswara.nagulendran@amd.com> > Cc: Felipe Clark <felipe.clark@amd.com> > Cc: Harry Wentland <Harry.Wentland@amd.com> > Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > include/drm/display/drm_dp.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/display/drm_dp.h > b/include/drm/display/drm_dp.h index 632376c291db..d30a9b2f450c 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP (1 << 4) > > #define DP_EDP_GENERAL_CAP_2 0x703 > # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0)
[AMD Official Use Only - General] Hi Rodrigo and Harry, There was a typo in the third line of the pseudo-patch I wrote in the previous email. Here is the fixed version: #define DP_EDP_GENERAL_CAP_2 0x703 # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0) +# define DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE (1<<4) # define DP_EDP_DYNAMIC_BACKLIGHT_ENABLE (1 << 4) # define DP_EDP_REGIONAL_BACKLIGHT_ENABLE (1 << 5) # define DP_EDP_UPDATE_REGION_BRIGHTNESS (1 << 6) /* eDP 1.4 */ +# define DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE (1<<7) #define DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET 0x732 #define DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET 0x733 +#define DP_EDP_PANEL_TARGET_LUMINANCE_VALUE 0x734 Thank you, Felipe -----Original Message----- From: Clark, Felipe Sent: Thursday, March 23, 2023 3:25 PM To: Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; airlied@gmail.com; daniel@ffwll.ch Cc: amd-gfx@lists.freedesktop.org; Koo, Anthony <Anthony.Koo@amd.com>; Nagulendran, Iswara <Iswara.Nagulendran@amd.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org Subject: RE: [PATCH] drm/display: Add missing OLED Vesa brightnesses definitions Hello Rodrigo and Harry, I would like to propose some changes to keep this patch consistent with the naming scheme and general organization of the drm_dp.h file. #define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 It would be better to use the (1<<7) representation for this bit to follow the pattern established by the other defines in the file. Also, a more generic name for this macro would be DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE. # define DP_EDP_OLED_VESA_CAP (1 << 4) A more generic name for this macro would be DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE In terms of the file structure, DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE should appear underneath the definition of DP_EDP_BACKLIGHT_MODE_SET_REGISTER. Similarly, DP_EDP_PANEL_LUMINANCE_CONTROL_CAPABLE should appear underneath the definition of DP_EDP_GENERAL_CAP_2 For a complete definition of the millinit based brightness control specification the following should also be added: #define DP_EDP_PANEL_TARGET_LUMINANCE_VALUE 0x734 Here is a suggested pseudo-patch with all these changes: #define DP_EDP_GENERAL_CAP_2 0x703 # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0) +# define DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE (1<<7) # define DP_EDP_DYNAMIC_BACKLIGHT_ENABLE (1 << 4) # define DP_EDP_REGIONAL_BACKLIGHT_ENABLE (1 << 5) # define DP_EDP_UPDATE_REGION_BRIGHTNESS (1 << 6) /* eDP 1.4 */ +# define DP_EDP_PANEL_LUMINANCE_CONTROL_ENABLE (1<<7) #define DP_EDP_DBC_MINIMUM_BRIGHTNESS_SET 0x732 #define DP_EDP_DBC_MAXIMUM_BRIGHTNESS_SET 0x733 +#define DP_EDP_PANEL_TARGET_LUMINANCE_VALUE 0x734 Thank you, Felipe -----Original Message----- From: Wentland, Harry <Harry.Wentland@amd.com> Sent: Wednesday, March 22, 2023 2:01 PM To: Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; airlied@gmail.com; daniel@ffwll.ch Cc: amd-gfx@lists.freedesktop.org; Koo, Anthony <Anthony.Koo@amd.com>; Nagulendran, Iswara <Iswara.Nagulendran@amd.com>; Clark, Felipe <Felipe.Clark@amd.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/display: Add missing OLED Vesa brightnesses definitions On 3/22/23 12:05, Rodrigo Siqueira wrote: > Cc: Anthony Koo <anthony.koo@amd.com> > Cc: Iswara Negulendran <iswara.nagulendran@amd.com> > Cc: Felipe Clark <felipe.clark@amd.com> > Cc: Harry Wentland <Harry.Wentland@amd.com> > Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > include/drm/display/drm_dp.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/display/drm_dp.h > b/include/drm/display/drm_dp.h index 632376c291db..d30a9b2f450c 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP (1 << 4) > > #define DP_EDP_GENERAL_CAP_2 0x703 > # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0)
On Wed 2023-03-22 10:05:13, Rodrigo Siqueira wrote: > Cc: Anthony Koo <anthony.koo@amd.com> > Cc: Iswara Negulendran <iswara.nagulendran@amd.com> > Cc: Felipe Clark <felipe.clark@amd.com> > Cc: Harry Wentland <Harry.Wentland@amd.com> > Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> Some changelog would be useful. > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP (1 << 4) > Are you fixing a compile problem? Otherwise this should go in with the user... BR, Pavel
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h index 632376c291db..d30a9b2f450c 100644 --- a/include/drm/display/drm_dp.h +++ b/include/drm/display/drm_dp.h @@ -977,6 +977,8 @@ # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) # define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 +# define DP_EDP_OLED_VESA_CAP (1 << 4) #define DP_EDP_GENERAL_CAP_2 0x703 # define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0)
Cc: Anthony Koo <anthony.koo@amd.com> Cc: Iswara Negulendran <iswara.nagulendran@amd.com> Cc: Felipe Clark <felipe.clark@amd.com> Cc: Harry Wentland <Harry.Wentland@amd.com> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> --- include/drm/display/drm_dp.h | 2 ++ 1 file changed, 2 insertions(+)