| Message ID | 1409626606-15225-1-git-send-email-djkurtz@chromium.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi Thierry, Congratulations! I saw in one thread that you said you are out on paternity leave. You mentioned that you would be back around v3.17-rc6, which should be soonish, so just a gentle ping on this patch for when you return. Thanks, -Daniel
On Sun, Sep 21, 2014 at 03:53:51PM +0800, Daniel Kurtz wrote: > Hi Thierry, > > Congratulations! I saw in one thread that you said you are out on > paternity leave. Thanks. > You mentioned that you would be back around v3.17-rc6, which should be > soonish, so just a gentle ping on this patch for when you return. I'm back to work starting today but catching up on a lot of email, so it might take me a little to get to the bottom of this. Thierry
On Tue, Sep 02, 2014 at 10:56:46AM +0800, Daniel Kurtz wrote: > There are several different models of N116BGE. According to the commit > that added innolux_n116bge_mode [0], this N116BGE is for the eDP variety. > > [0] commit 0a2288c06aab73c966e82045c8f20b0e713baf2a > Author: Thierry Reding <treding@nvidia.com> > Date: Thu Jul 3 14:02:59 2014 +0200 > > drm/panel: simple: Add Innolux N116BGE panel support > > The clock and htotal values from add by that patch are out of spec according to > the datasheets I have seen for the eDP N116BGE (-EA2 and -EB2). Does out of spec imply that these timings aren't working for you? > This patch changes the values to the "Typ" values on the datasheet. The original patch was based on -E42 of the datasheet. But I can run some tests to see if the timings in this patch work on the panel that I have. If so I guess the easiest would be to apply this. It shouldn't matter all that much since the panel has EDID if I remember correctly. The modes parsed from EDID should take precedence over the fixed mode. If they don't then that's a bug. Thierry > > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> > --- > > Thierry, > > It is possible that your timings were correct for the panel you are using on > the norrin reference board. In that case I'm happy to upload a new patch > that creates a new panel entry. However, I'm pretty sure we are using the > same N116BGE. > > drivers/gpu/drm/panel/panel-simple.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c > index 4ce1db0..776764a 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -519,15 +519,15 @@ static const struct panel_desc foxlink_fl500wvr00_a0t = { > }; > > static const struct drm_display_mode innolux_n116bge_mode = { > - .clock = 71000, > + .clock = 76420, > .hdisplay = 1366, > - .hsync_start = 1366 + 64, > - .hsync_end = 1366 + 64 + 6, > - .htotal = 1366 + 64 + 6 + 64, > + .hsync_start = 1366 + 136, > + .hsync_end = 1366 + 136 + 30, > + .htotal = 1366 + 136 + 30 + 60, > .vdisplay = 768, > .vsync_start = 768 + 8, > - .vsync_end = 768 + 8 + 4, > - .vtotal = 768 + 8 + 4 + 8, > + .vsync_end = 768 + 8 + 12, > + .vtotal = 768 + 8 + 12 + 12, > .vrefresh = 60, > .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, > }; > -- > 2.1.0.rc2.206.gedb03e5 >
On Mon, Sep 22, 2014 at 4:39 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > > On Tue, Sep 02, 2014 at 10:56:46AM +0800, Daniel Kurtz wrote: > > There are several different models of N116BGE. According to the commit > > that added innolux_n116bge_mode [0], this N116BGE is for the eDP variety. > > > > [0] commit 0a2288c06aab73c966e82045c8f20b0e713baf2a > > Author: Thierry Reding <treding@nvidia.com> > > Date: Thu Jul 3 14:02:59 2014 +0200 > > > > drm/panel: simple: Add Innolux N116BGE panel support > > > > The clock and htotal values from add by that patch are out of spec according to > > the datasheets I have seen for the eDP N116BGE (-EA2 and -EB2). > > Does out of spec imply that these timings aren't working for you? No, I didn't even try with your values, actually. Out of spec just means that the numbers do not fall between Min and Max on the datasheet: 72.6 <= DCLK <= 80.24 vs. 71 1506 <= htotal <= 1716 vs. 1500 > > This patch changes the values to the "Typ" values on the datasheet. > > The original patch was based on -E42 of the datasheet. But I can run > some tests to see if the timings in this patch work on the panel that I > have. If so I guess the easiest would be to apply this. Thanks! > > It shouldn't matter all that much since the panel has EDID if I remember > correctly. The modes parsed from EDID should take precedence over the > fixed mode. If they don't then that's a bug. The driver I am testing this with does not yet support reading the EDID over eDP, so I am using the fixed values for now. > Thierry >
On Mon, Sep 22, 2014 at 06:12:41PM +0800, Daniel Kurtz wrote: > On Mon, Sep 22, 2014 at 4:39 PM, Thierry Reding <thierry.reding@gmail.com> wrote: > > It shouldn't matter all that much since the panel has EDID if I remember > > correctly. The modes parsed from EDID should take precedence over the > > fixed mode. If they don't then that's a bug. > > The driver I am testing this with does not yet support reading the > EDID over eDP, so I am using the fixed values for now. Ew... you should get a better driver, then. =) Thierry
On Mon, Sep 22, 2014 at 06:12:41PM +0800, Daniel Kurtz wrote: > On Mon, Sep 22, 2014 at 4:39 PM, Thierry Reding > <thierry.reding@gmail.com> wrote: > > > > On Tue, Sep 02, 2014 at 10:56:46AM +0800, Daniel Kurtz wrote: > > > There are several different models of N116BGE. According to the commit > > > that added innolux_n116bge_mode [0], this N116BGE is for the eDP variety. > > > > > > [0] commit 0a2288c06aab73c966e82045c8f20b0e713baf2a > > > Author: Thierry Reding <treding@nvidia.com> > > > Date: Thu Jul 3 14:02:59 2014 +0200 > > > > > > drm/panel: simple: Add Innolux N116BGE panel support > > > > > > The clock and htotal values from add by that patch are out of spec according to > > > the datasheets I have seen for the eDP N116BGE (-EA2 and -EB2). > > > > Does out of spec imply that these timings aren't working for you? > > No, I didn't even try with your values, actually. > Out of spec just means that the numbers do not fall between Min and > Max on the datasheet: > > 72.6 <= DCLK <= 80.24 vs. 71 > 1506 <= htotal <= 1716 vs. 1500 > > > > This patch changes the values to the "Typ" values on the datasheet. > > > > The original patch was based on -E42 of the datasheet. But I can run > > some tests to see if the timings in this patch work on the panel that I > > have. If so I guess the easiest would be to apply this. > > Thanks! Took me a while longer to test this than I had expected. Turns out the Tegra132 Norrin board that I have works with the timings you provide in this patch too, so I'm going to apply this patch. Thierry
diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index 4ce1db0..776764a 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -519,15 +519,15 @@ static const struct panel_desc foxlink_fl500wvr00_a0t = { }; static const struct drm_display_mode innolux_n116bge_mode = { - .clock = 71000, + .clock = 76420, .hdisplay = 1366, - .hsync_start = 1366 + 64, - .hsync_end = 1366 + 64 + 6, - .htotal = 1366 + 64 + 6 + 64, + .hsync_start = 1366 + 136, + .hsync_end = 1366 + 136 + 30, + .htotal = 1366 + 136 + 30 + 60, .vdisplay = 768, .vsync_start = 768 + 8, - .vsync_end = 768 + 8 + 4, - .vtotal = 768 + 8 + 4 + 8, + .vsync_end = 768 + 8 + 12, + .vtotal = 768 + 8 + 12 + 12, .vrefresh = 60, .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC, };
There are several different models of N116BGE. According to the commit that added innolux_n116bge_mode [0], this N116BGE is for the eDP variety. [0] commit 0a2288c06aab73c966e82045c8f20b0e713baf2a Author: Thierry Reding <treding@nvidia.com> Date: Thu Jul 3 14:02:59 2014 +0200 drm/panel: simple: Add Innolux N116BGE panel support The clock and htotal values from add by that patch are out of spec according to the datasheets I have seen for the eDP N116BGE (-EA2 and -EB2). This patch changes the values to the "Typ" values on the datasheet. Signed-off-by: Daniel Kurtz <djkurtz@chromium.org> --- Thierry, It is possible that your timings were correct for the panel you are using on the norrin reference board. In that case I'm happy to upload a new patch that creates a new panel entry. However, I'm pretty sure we are using the same N116BGE. drivers/gpu/drm/panel/panel-simple.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)