| Message ID | 1624099230-20899-1-git-send-email-rajeevny@codeaurora.org |
|---|---|
| Headers | show |
| Series | drm: Support basic DPCD backlight in panel-simple and add a new panel ATNA33XC20 | expand |
Hi, On Sun, Jun 20, 2021 at 3:01 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Rajeev > On Sat, Jun 19, 2021 at 04:10:30PM +0530, Rajeev Nandan wrote: > > Add Samsung 13.3" FHD eDP AMOLED panel. > > > > Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > --- > > > > Changes in v4: > > - New > > > > Changes in v5: > > - Remove "uses_dpcd_backlight" property, not required now. (Douglas) > > > > Changes in v7: > > - Update disable_to_power_off and power_to_enable delays. (Douglas) > > > > drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > index 86e5a45..4adc44a 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = { > > .connector_type = DRM_MODE_CONNECTOR_LVDS, > > }; > > > > +static const struct drm_display_mode samsung_atna33xc20_mode = { > > + .clock = 138770, > > + .hdisplay = 1920, > > + .hsync_start = 1920 + 48, > > + .hsync_end = 1920 + 48 + 32, > > + .htotal = 1920 + 48 + 32 + 80, > > + .vdisplay = 1080, > > + .vsync_start = 1080 + 8, > > + .vsync_end = 1080 + 8 + 8, > > + .vtotal = 1080 + 8 + 8 + 16, > > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC, > > +}; > > + > > +static const struct panel_desc samsung_atna33xc20 = { > > + .modes = &samsung_atna33xc20_mode, > > + .num_modes = 1, > > + .bpc = 10, > > + .size = { > > + .width = 294, > > + .height = 165, > > + }, > > + .delay = { > > + .disable_to_power_off = 200, > > + .power_to_enable = 400, > > + .hpd_absent_delay = 200, > > + .unprepare = 500, > > + }, > > + .connector_type = DRM_MODE_CONNECTOR_eDP, > > +}; > > bus_format is missing. There should be a warning about this when you > probe the display. Sam: I'm curious about the requirement of hardcoding bus_format like this for eDP panels. Most eDP panels support a variety of bits per pixel and do so dynamically. Ones I've poked at freely support 6bpp and 8bpp. Presumably this one supports both of those modes and also 10bpp. I haven't done detailed research on it, but it would also surprise me if the "bus format" for a given bpp needed to be specified for eDP. Presumably since eDP has most of the "autodetect" type features of DP then if the format needed to be accounted for that you could query the hardware? Looking at the datasheet for the ti-sn65dsi86 MIPI-to-eDP bridge chip I see that it explicitly calls out the bus formats that it supports for the MIPI side but doesn't call out anything for eDP. That would tend to support my belief that there isn't variance on the eDP side... Maybe the right fix is to actually change the check not to give a warning for eDP panels? ...or am I misunderstanding? > The bpc of 10 in unusual, the current code warns if bpc is neither 6 nor > 8. If 10 is correct then update the code to accept bpc=10. I'm pretty sure it's 10 based on this panel's datasheet, though this panel also accepts 8 bpc. Fixing the warning seems like a good idea to me--I wasn't aware of it. -Doug
Hi, On Mon, Jun 21, 2021 at 11:42 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > Hi Doug, > > On Mon, Jun 21, 2021 at 08:34:51AM -0700, Doug Anderson wrote: > > Hi, > > > > On Sun, Jun 20, 2021 at 3:01 AM Sam Ravnborg <sam@ravnborg.org> wrote: > > > > > > Hi Rajeev > > > On Sat, Jun 19, 2021 at 04:10:30PM +0530, Rajeev Nandan wrote: > > > > Add Samsung 13.3" FHD eDP AMOLED panel. > > > > > > > > Signed-off-by: Rajeev Nandan <rajeevny@codeaurora.org> > > > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > > > --- > > > > > > > > Changes in v4: > > > > - New > > > > > > > > Changes in v5: > > > > - Remove "uses_dpcd_backlight" property, not required now. (Douglas) > > > > > > > > Changes in v7: > > > > - Update disable_to_power_off and power_to_enable delays. (Douglas) > > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 33 +++++++++++++++++++++++++++++++++ > > > > 1 file changed, 33 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > > > > index 86e5a45..4adc44a 100644 > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > @@ -3562,6 +3562,36 @@ static const struct panel_desc rocktech_rk101ii01d_ct = { > > > > .connector_type = DRM_MODE_CONNECTOR_LVDS, > > > > }; > > > > > > > > +static const struct drm_display_mode samsung_atna33xc20_mode = { > > > > + .clock = 138770, > > > > + .hdisplay = 1920, > > > > + .hsync_start = 1920 + 48, > > > > + .hsync_end = 1920 + 48 + 32, > > > > + .htotal = 1920 + 48 + 32 + 80, > > > > + .vdisplay = 1080, > > > > + .vsync_start = 1080 + 8, > > > > + .vsync_end = 1080 + 8 + 8, > > > > + .vtotal = 1080 + 8 + 8 + 16, > > > > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NVSYNC, > > > > +}; > > > > + > > > > +static const struct panel_desc samsung_atna33xc20 = { > > > > + .modes = &samsung_atna33xc20_mode, > > > > + .num_modes = 1, > > > > + .bpc = 10, > > > > + .size = { > > > > + .width = 294, > > > > + .height = 165, > > > > + }, > > > > + .delay = { > > > > + .disable_to_power_off = 200, > > > > + .power_to_enable = 400, > > > > + .hpd_absent_delay = 200, > > > > + .unprepare = 500, > > > > + }, > > > > + .connector_type = DRM_MODE_CONNECTOR_eDP, > > > > +}; > > > > > > bus_format is missing. There should be a warning about this when you > > > probe the display. > > > > Sam: I'm curious about the requirement of hardcoding bus_format like > > this for eDP panels. Most eDP panels support a variety of bits per > > pixel and do so dynamically. Ones I've poked at freely support 6bpp > > and 8bpp. Presumably this one supports both of those modes and also > > 10bpp. I haven't done detailed research on it, but it would also > > surprise me if the "bus format" for a given bpp needed to be specified > > for eDP. Presumably since eDP has most of the "autodetect" type > > features of DP then if the format needed to be accounted for that you > > could query the hardware? > > > > Looking at the datasheet for the ti-sn65dsi86 MIPI-to-eDP bridge chip > > I see that it explicitly calls out the bus formats that it supports > > for the MIPI side but doesn't call out anything for eDP. That would > > tend to support my belief that there isn't variance on the eDP side... > > > > Maybe the right fix is to actually change the check not to give a > > warning for eDP panels? ...or am I misunderstanding? > > I have never dived into the datasheets of eDP panels so I do not know. > The checks were added based on what we had in-tree and it is no suprise > if they need an update or are just plain wrong. > I expect you to be in a better position to make the call here - but we > should not add panels that triggers warnings so either fix the warnings > or fix the panel description. Agreed. I'd support a patch that removes this warning for eDP panels unless someone knows that it makes sense. I haven't been able to find anything indicating that it does. -Doug