On Wed, 21 Oct 2015, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Tue, Oct 20, 2015 at 03:38:33PM +0300, Jani Nikula wrote: >> Have only one if ladder for platforms and only one range check for >> size. Makes it easier to handle new platforms. Remove the use of >> negative return values in char, which might underflow to be positive for >> some negative error codes. >> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > On the series: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> Apparently this conflicts with some other firmware patches. I'll let Imre decide if we should give them priority, or just merge these. BR, Jani. > >> --- >> drivers/gpu/drm/i915/intel_csr.c | 46 ++++++++++++++++++++-------------------- >> 1 file changed, 23 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c >> index fe9a55cf8212..3e7953254803 100644 >> --- a/drivers/gpu/drm/i915/intel_csr.c >> +++ b/drivers/gpu/drm/i915/intel_csr.c >> @@ -188,28 +188,25 @@ static const struct stepping_info bxt_stepping_info[] = { >> {'B', '0'}, {'B', '1'}, {'B', '2'} >> }; >> >> -static char intel_get_stepping(struct drm_device *dev) >> +static const struct stepping_info *intel_get_stepping_info(struct drm_device *dev) >> { >> - if (IS_SKYLAKE(dev) && (dev->pdev->revision < >> - ARRAY_SIZE(skl_stepping_info))) >> - return skl_stepping_info[dev->pdev->revision].stepping; >> - else if (IS_BROXTON(dev) && (dev->pdev->revision < >> - ARRAY_SIZE(bxt_stepping_info))) >> - return bxt_stepping_info[dev->pdev->revision].stepping; >> - else >> - return -ENODATA; >> -} >> + const struct stepping_info *si; >> + unsigned int size; >> + >> + if (IS_SKYLAKE(dev)) { >> + size = ARRAY_SIZE(skl_stepping_info); >> + si = skl_stepping_info; >> + } else if (IS_BROXTON(dev)) { >> + size = ARRAY_SIZE(bxt_stepping_info); >> + si = bxt_stepping_info; >> + } else { >> + return NULL; >> + } >> >> -static char intel_get_substepping(struct drm_device *dev) >> -{ >> - if (IS_SKYLAKE(dev) && (dev->pdev->revision < >> - ARRAY_SIZE(skl_stepping_info))) >> - return skl_stepping_info[dev->pdev->revision].substepping; >> - else if (IS_BROXTON(dev) && (dev->pdev->revision < >> - ARRAY_SIZE(bxt_stepping_info))) >> - return bxt_stepping_info[dev->pdev->revision].substepping; >> - else >> - return -ENODATA; >> + if (INTEL_REVID(dev) < size) >> + return si + INTEL_REVID(dev); >> + >> + return NULL; >> } >> >> /** >> @@ -296,8 +293,8 @@ static void finish_csr_load(const struct firmware *fw, void *context) >> struct intel_package_header *package_header; >> struct intel_dmc_header *dmc_header; >> struct intel_csr *csr = &dev_priv->csr; >> - char stepping = intel_get_stepping(dev); >> - char substepping = intel_get_substepping(dev); >> + const struct stepping_info *stepping_info = intel_get_stepping_info(dev); >> + char stepping, substepping; >> uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes; >> uint32_t i; >> uint32_t *dmc_payload; >> @@ -308,11 +305,14 @@ static void finish_csr_load(const struct firmware *fw, void *context) >> goto out; >> } >> >> - if ((stepping == -ENODATA) || (substepping == -ENODATA)) { >> + if (!stepping_info) { >> DRM_ERROR("Unknown stepping info, firmware loading failed\n"); >> goto out; >> } >> >> + stepping = stepping_info->stepping; >> + substepping = stepping_info->substepping; >> + >> /* Extract CSS Header information*/ >> css_header = (struct intel_css_header *)fw->data; >> if (sizeof(struct intel_css_header) != >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx