From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758118AbcBDPWc (ORCPT ); Thu, 4 Feb 2016 10:22:32 -0500 Received: from smtprelay.synopsys.com ([198.182.47.9]:40432 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756597AbcBDPWa (ORCPT ); Thu, 4 Feb 2016 10:22:30 -0500 Subject: Re: [PATCH] adv7511: Added mode_fixup function. To: Laurent Pinchart , Carlos Palminha , David Airlie References: <1454063627-12219-1-git-send-email-palminha@synopsys.com> <1948106.WC7MId72vp@avalon> <56AF5176.80603@synopsys.com> CC: , , Laurent Pinchart From: Carlos Palminha Message-ID: <56B36CB1.80000@synopsys.com> Date: Thu, 4 Feb 2016 15:22:25 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56AF5176.80603@synopsys.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.107.19.98] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi guys, any feedback? patch will be accepted for adv7511 driver? Regards, C.Palminha On 01-02-2016 12:37, Carlos Palminha wrote: > Hi Laurent > > On 29-01-2016 17:48, Laurent Pinchart wrote: >> Hi Carlos, >> >> Thank you for the patch. >> >> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: >>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. >> >> Instead of implementing stubs in encoder drivers, wouldn't it be better to >> make mode_fixup optional ? > Probably you are right but i don't have enough knowledge or time to do that for the DRM framework. :( > I limited myself to do what the other drivers already implement. > > The patch is mandatory to have to the ADV working or else will get some NULL pointer crash. > > Regards, > C.Palminha > >> >>> Signed-off-by: Carlos Palminha >>> --- >>> drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c >>> index 533d1e3..90082d2 100644 >>> --- a/drivers/gpu/drm/i2c/adv7511.c >>> +++ b/drivers/gpu/drm/i2c/adv7511.c >>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, >>> return status; >>> } >>> >>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, >>> + const struct drm_display_mode *mode, >>> + struct drm_display_mode *adjusted_mode) >>> +{ >>> + return true; >>> +} >>> + >>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, >>> struct drm_display_mode *mode) >>> { >>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder >>> *encoder, >>> >>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { >>> .dpms = adv7511_encoder_dpms, >>> + .mode_fixup = adv7511_encoder_mode_fixup, >>> .mode_valid = adv7511_encoder_mode_valid, >>> .mode_set = adv7511_encoder_mode_set, >>> .detect = adv7511_encoder_detect, >> > > > On 29-01-2016 17:48, Laurent Pinchart wrote: >> Hi Carlos, >> >> Thank you for the patch. >> >> On Friday 29 January 2016 10:33:47 Carlos Palminha wrote: >>> The mode_fixup is necessary when using it in a DRM FB driver pipeline. >> >> Instead of implementing stubs in encoder drivers, wouldn't it be better to >> make mode_fixup optional ? >> >>> Signed-off-by: Carlos Palminha >>> --- >>> drivers/gpu/drm/i2c/adv7511.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i2c/adv7511.c b/drivers/gpu/drm/i2c/adv7511.c >>> index 533d1e3..90082d2 100644 >>> --- a/drivers/gpu/drm/i2c/adv7511.c >>> +++ b/drivers/gpu/drm/i2c/adv7511.c >>> @@ -648,6 +648,13 @@ adv7511_encoder_detect(struct drm_encoder *encoder, >>> return status; >>> } >>> >>> +static bool adv7511_encoder_mode_fixup(struct drm_encoder *encoder, >>> + const struct drm_display_mode *mode, >>> + struct drm_display_mode *adjusted_mode) >>> +{ >>> + return true; >>> +} >>> + >>> static int adv7511_encoder_mode_valid(struct drm_encoder *encoder, >>> struct drm_display_mode *mode) >>> { >>> @@ -754,6 +761,7 @@ static void adv7511_encoder_mode_set(struct drm_encoder >>> *encoder, >>> >>> static const struct drm_encoder_slave_funcs adv7511_encoder_funcs = { >>> .dpms = adv7511_encoder_dpms, >>> + .mode_fixup = adv7511_encoder_mode_fixup, >>> .mode_valid = adv7511_encoder_mode_valid, >>> .mode_set = adv7511_encoder_mode_set, >>> .detect = adv7511_encoder_detect, >>