From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915/hdmi: Compute port_clock for 27.027 pixel replicated modes Date: Tue, 7 Oct 2014 11:29:30 +0300 Message-ID: <20141007082930.GQ32511@intel.com> References: <1411598979-4290-1-git-send-email-clinton.a.taylor@intel.com> <20140926155838.GA32511@intel.com> <54259442.60307@intel.com> <20140930124654.GK32511@intel.com> <54332323.10204@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 3E5BF89F85 for ; Tue, 7 Oct 2014 01:29:34 -0700 (PDT) Content-Disposition: inline In-Reply-To: <54332323.10204@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Clint Taylor Cc: Intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Oct 06, 2014 at 04:17:55PM -0700, Clint Taylor wrote: > On 09/30/2014 05:46 AM, Ville Syrj=E4l=E4 wrote: > > On Fri, Sep 26, 2014 at 09:28:50AM -0700, Clint Taylor wrote: > >> On 09/26/2014 08:58 AM, Ville Syrj=E4l=E4 wrote: > >>> On Wed, Sep 24, 2014 at 03:49:39PM -0700, clinton.a.taylor@intel.com = wrote: > >>>> From: Clint Taylor > >>>> > >>>> port_clock was being incorrectly computed and WRPLL was incorrectly > >>>> programmed for pixel doubled modes using a 27.027MHz pixel clock. > >>>> port_clock was set to 27.026 resulting in an output pixel clock > >>>> matching 27.000MHz. > >>> > >>> I don't understand how this 27.000MHz clock would come about. Can you > >>> explain further? > >> > >> I didn't really drill down into the issue once I saw the WRPLL was bei= ng > >> computed differently for 480i@60 and the port clock was being passed as > >> 27026. > >> > >>> > >>>> Since there is no way to correctly half the 27.027 > >>>> frequency as an integer just set port_clock to 27027 for these modes. > >>>> > >>>> Signed-off-by: Clint Taylor > >>>> --- > >>>> drivers/gpu/drm/i915/intel_hdmi.c | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i91= 5/intel_hdmi.c > >>>> index 29ec153..a0786d6 100644 > >>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c > >>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c > >>>> @@ -933,6 +933,11 @@ bool intel_hdmi_compute_config(struct intel_enc= oder *encoder, > >>>> > >>>> if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) { > >>>> pipe_config->pixel_multiplier =3D 2; > >>>> + > >>>> + /* Fix up port_clock since 27027 is not divisible by 2 */ > >>>> + if (pipe_config->adjusted_mode.crtc_clock =3D=3D 13513) { > >>>> + pipe_config->port_clock =3D 27027; > >>>> + } > >>> > >>> Ugh. Oh well I suppose we'll want to be as accurate as possible. Would > >>> be nice if would could someone do this fixup already in drm core, but > >>> that would probably require changing to smaller clock units all over > >>> the place which doesn't sound fun at all. So I guess we get to leave > >>> the kludge in the driver :( > >> > >> I don't like using the 13MHz family in the mode table for this very > >> reason. Maybe I should revisit the pixel clock setting and computation > >> for pixel doubled modes in the i915 code. > > > > That has certain appeal, but it will force userspace to adjust the > > clock for double clocked modes when it wants to calculate the refresh > > rate. I don't really like even exposing the DBLCLK flag to userspace > > since it's basically an internal implementation detail. Some of the > > modes also allow higher pixel repeat factors, and we have no way to > > inform userspace about those. So I think that forcing userspace to > > deal with the multiplied clock is not a good idea. > > > >> > >>> > >>> This also got me thinking about the 12bpc case as well. It's clearly > >>> busted with double clocked modes now. Care to fix that up as well? > >> > >> What's the best way to enter into 12bpc mode? I tried to force the mode > >> in hdmi_compute_config(), but I wasn't able to get output. > > > > Looks like our 12bpc suppor is pretty broken. I did manage to make it > > work on my IVB here but that was after I wrote several patches to fix > > things. I don't have time to clean those up right now, but if anyone > > else wants to work on that I can share them. > = > Send me the patches and I can look at the 12bpc support. My main concern = > with 12bpc is we don't automatically switch to 12 bit when the HDMI = > monitor supports it and nobody would take the time to manually switch to = > 12bpc using an x11 property every time they boot. I've never had an = > issue with sending Deep Color to a TV that supports it, but I'm also not = > ready to try and make it automatic in the driver. We do it automagically but we make the decision based on the primary plane pixel format which is a bit silly considering there are sprites and whatnot to consider. Maybe we should just always go for deep color whenever possible? Anyway that's a separate topic. I did manage to clean up the patches a bit while I had nothing else to do. I'll give them a quick spin and if they still work I'll toss them onto the mailing list. > = > -Clint > = > > > >> > >>> > >>> PS. did you ever send that separate patch to fix up the interlace > >>> flags in some of the double clocked modes? I don't remember seeing > >>> such a patch at least. > >> > >> I had that on my TODO list and when I went to look at the modes (576i > >> @200Hz) they were already correct. Magic happens!! > >> > >> -Clint > >>> > >>>> } > >>>> > >>>> if (intel_hdmi->color_range) > >>>> -- > >>>> 1.7.9.5 > >>>> > >>>> _______________________________________________ > >>>> Intel-gfx mailing list > >>>> Intel-gfx@lists.freedesktop.org > >>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >>> > > -- = Ville Syrj=E4l=E4 Intel OTC